Handle Authentication Error #1062
Merged
+139
−78
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TristanCacqueray
force-pushed
the
auth-error
branch
from
3b5d938 to
d8007cc
Compare
|
@morucci I've replaced the streaming error handling with a simpler approach in this PR. Though I don't expect it to fix the issue, and if it does, it would be good to know why. What we observed is that for take datas, the processing ends correctly: But for projects and pull-requests, the error is somehow ignored: |
TristanCacqueray
force-pushed
the
auth-error
branch
4 times, most recently
from
5db727c to
9ce14a6
Compare
This change avoids retrying requests that fail with a 401 error.
This change simplifies the stream handling by: - Using an explicit return value instead of a throw exception, - Handwritten process batch stream consumer.
|
@morucci in the last commit I've also added a test to validate the logic, and the test pass with or without the refactor, so I think the issue is somewhere else in the GraphQL module. |
|
Thanks for the refactor. Let's merge it. However yes it is a weird that processStream do not receive the GraphError from streamFetch. |
morucci
approved these changes
Sep 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change avoids retrying requests that fail with a 401 error. #1060