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 an error when checkout 404s. indicate to client that it may be… #42

Merged
merged 4 commits into from
Nov 2, 2023

Conversation

cianbuckley
Copy link
Contributor

@cianbuckley cianbuckley commented Oct 24, 2023

… due to store using checkout.liquid

What are you trying to accomplish?

When a shop is still on checkout.liquid, server returns a 404 as the sdk is not compatible with single page checkout. That's not been very clear to users of the sdk, so this PR throws a more descriptive stack trace and documents it

Before you deploy

  • I have added tests to support my implementation
  • I have read and agree with the contributing documentation readme
  • I have read and agree with the code of conduct documentation readme
  • I have updated any documentation related to these changes.
  • I have updated the README (if applicable).

@cianbuckley cianbuckley force-pushed the error-on-checkout-liquid branch from a058e24 to 178a52c Compare October 30, 2023 09:02
@cianbuckley cianbuckley marked this pull request as ready for review October 31, 2023 08:18
@cianbuckley cianbuckley requested a review from a team as a code owner October 31, 2023 08:18
@cianbuckley cianbuckley requested a review from markmur October 31, 2023 10:41
Comment on lines 150 to 151
case 404:
viewDelegate?.checkoutViewDidFailWithError(error: .sdkError(underlying: CheckoutLiquidError.unmigratedCheckoutError(message: "The checkout url provided has resulted in an error. The store is still using checkout.liquid, whereas the checkout SDK only supports checkout with extensibility.")))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it a guarantee that 404 = unmigrated? What if the cart/checkout simply doesn't exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just not great is it.. I'll move the needle on a more appropriate status code response over the coming days

@cianbuckley cianbuckley force-pushed the error-on-checkout-liquid branch from 667b005 to f557ade Compare November 2, 2023 16:32
@cianbuckley cianbuckley merged commit 9ae3dc8 into main Nov 2, 2023
4 checks passed
@cianbuckley cianbuckley deleted the error-on-checkout-liquid branch November 2, 2023 16:36
markmur pushed a commit that referenced this pull request Nov 7, 2023
#42)

* return an error when checkout 404s. indicate to client that it may be due to store using checkout.liquid

* remove references to 404 favouring client facing error

* favour flattened error hierarchy

* linting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants