Skip to content

fix: shop is not reachable when getting 401 unauthorized response #58

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

Conversation

MoritzKrafeld
Copy link
Member

@MoritzKrafeld MoritzKrafeld commented Jan 17, 2025

This PR started with this discussion: #56 (comment)
I tested it with multiple shops and can confirm that without these changes, some of them would not be able to pass this check as they might return 401 instead of 200. With this change, I was not able to find a shop where 401 would not pass this check.

@MoritzKrafeld MoritzKrafeld added the bug Something isn't working label Jan 17, 2025
@MoritzKrafeld MoritzKrafeld force-pushed the fix-shop-is-not-reachable-when-getting-401-unauthorized-response branch from 83b6de3 to 79eb123 Compare January 17, 2025 15:46
Comment on lines 44 to 45
if ($e instanceof HttpException && $e->getStatusCode() === Response::HTTP_UNAUTHORIZED) {
return;
}

if ($e instanceof HttpExceptionInterface && $e->getResponse()->getStatusCode() === Response::HTTP_UNAUTHORIZED) {
return;
}

Copy link
Contributor

@nsaliu nsaliu Jan 17, 2025

Choose a reason for hiding this comment

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

Hey @MoritzKrafeld, what if we only check if $e is an instance of TransportExceptionInterface ?

If it is, this means we have a "network error" and the host is not reachable.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're 100% right! I changed it so that we only throw the exception if the exception implements your mentioned TransportExceptionInterface 😄

@MoritzKrafeld MoritzKrafeld force-pushed the fix-shop-is-not-reachable-when-getting-401-unauthorized-response branch from 79eb123 to 7b06aa6 Compare January 20, 2025 07:09
@MoritzKrafeld MoritzKrafeld merged commit 1d6fbfe into main Jan 20, 2025
9 checks passed
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.

3 participants