Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return problem detail when LCP fulfillment fails (PP-1641) #2033

Merged
merged 1 commit into from
Sep 6, 2024

Conversation

jonathangreen
Copy link
Member

@jonathangreen jonathangreen commented Sep 5, 2024

Description

If LCP fulfillment request does not return a status in the 2xx range, raise a ProblemDetailException, which will be returned to the clients.

Motivation and Context

We've seen a number of times recently when LCP fulfillment fails, and the CM just blindly passes along whatever the LCP API gives us, which confuses the apps. Instead in this PR if we get an unexpected status code, we turn that into a problem detail, and log the issue. This should give a better user experience and let us track down the problems from our logs.

This PR is only a couple lines, but it took a lot of refactoring in #2034 and #2035 to get here.

How Has This Been Tested?

  • Tested locally

Checklist

  • I have updated the documentation accordingly.
  • All new and existing tests passed.

Copy link

codecov bot commented Sep 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.75%. Comparing base (5fb1411) to head (ba24269).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2033   +/-   ##
=======================================
  Coverage   90.75%   90.75%           
=======================================
  Files         342      342           
  Lines       40516    40516           
  Branches     8771     8771           
=======================================
  Hits        36772    36772           
  Misses       2486     2486           
  Partials     1258     1258           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jonathangreen jonathangreen force-pushed the feature/lcp-fulfillment branch from d204258 to c19d6bb Compare September 6, 2024 13:15
@jonathangreen jonathangreen changed the base branch from main to chore/refactor-bad-response-exception September 6, 2024 13:15
@jonathangreen jonathangreen force-pushed the feature/lcp-fulfillment branch from c5fd858 to 0925c1b Compare September 6, 2024 15:00
@jonathangreen jonathangreen changed the title Return problem detail when LCP fulfillment fails Return problem detail when LCP fulfillment fails (PP-1641) Sep 6, 2024
@jonathangreen jonathangreen force-pushed the feature/lcp-fulfillment branch from 0925c1b to 22154d0 Compare September 6, 2024 15:16
@jonathangreen jonathangreen changed the base branch from chore/refactor-bad-response-exception to chore/refactor-fulfillment September 6, 2024 15:17
@jonathangreen jonathangreen marked this pull request as ready for review September 6, 2024 15:22
@jonathangreen jonathangreen requested a review from a team September 6, 2024 15:22
@jonathangreen jonathangreen added the bug Something isn't working label Sep 6, 2024
Copy link
Contributor

@tdilauro tdilauro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏁

@jonathangreen jonathangreen force-pushed the chore/refactor-fulfillment branch from 1fdb345 to d166625 Compare September 6, 2024 16:23
@jonathangreen jonathangreen force-pushed the feature/lcp-fulfillment branch from 22154d0 to 12c4969 Compare September 6, 2024 16:26
Base automatically changed from chore/refactor-fulfillment to main September 6, 2024 17:50
Raise an exception if we get an unexpected status code while doing
a LCP fulfillment.
@jonathangreen jonathangreen force-pushed the feature/lcp-fulfillment branch from 12c4969 to ba24269 Compare September 6, 2024 17:51
@jonathangreen jonathangreen merged commit 9db1eeb into main Sep 6, 2024
20 checks passed
@jonathangreen jonathangreen deleted the feature/lcp-fulfillment branch September 6, 2024 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants