-
-
Notifications
You must be signed in to change notification settings - Fork 749
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
Gracefully handle subgraph transport errors #6968
Conversation
Qodana for .NET5 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at [email protected]
|
5c8b87e
to
5653f05
Compare
@@ -100,8 +98,7 @@ public ValueTask<OperationResult> ReadAsResultAsync(CancellationToken cancellati | |||
#endif | |||
} | |||
|
|||
// if the media type is anything else we will return a transport error. | |||
return new ValueTask<OperationResult>(_transportError); | |||
throw new Exception("Transport error"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throw a proper exception and why was the previous behavior even there?
@@ -187,7 +184,7 @@ public IAsyncEnumerable<OperationResult> ReadAsResultStreamAsync(CancellationTok | |||
#endif | |||
} | |||
|
|||
return SingleResult(new ValueTask<OperationResult>(_transportError)); | |||
throw new Exception("Stream transport error"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing
using var response = await _client.SendAsync(request, ct).ConfigureAwait(false); | ||
var result = await response.ReadAsResultAsync(ct).ConfigureAwait(false); | ||
return new GraphQLResponse(result); | ||
} | ||
catch | ||
{ | ||
return _transportError; | ||
return new GraphQLResponse(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing
HasError = hasError; | ||
} | ||
|
||
public bool HasError { get; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not correct, as there can be both successful and unsuccessful data as part of a single SelectionData.
Fixes #6752