-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Make protocol errors available in the error link #12281
Changes from 4 commits
445017e
7b4338c
e51ef3e
034dd47
e54cfda
6bc3346
23f4d7e
1fbf96c
7549224
5df6452
11f22f2
5ce2c00
17b7ed2
d5d4a77
e16cd54
266c642
cb5894f
bb99ec4
fefd26e
d5f2469
b1e73a2
40cd6f0
27d2eac
c7dde9e
fe6281c
4775b82
fc04917
d2f62ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
--- | ||
"@apollo/client": patch | ||
--- | ||
|
||
Make `protocolErrors` from multipart subscriptions available to the error link with the `protocolErrors` property. | ||
|
||
```js | ||
const errorLink = onError(({ protocolErrors }) => { | ||
if (protocolErrors) { | ||
console.log(protocolErrors); | ||
} | ||
}); | ||
``` |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,5 +1,13 @@ | ||||||
import type { FormattedExecutionResult, GraphQLFormattedError } from "graphql"; | ||||||
import type { | ||||||
FormattedExecutionResult, | ||||||
GraphQLErrorExtensions, | ||||||
GraphQLFormattedError, | ||||||
} from "graphql"; | ||||||
|
||||||
import { | ||||||
graphQLResultHasProtocolErrors, | ||||||
PROTOCOL_ERRORS_SYMBOL, | ||||||
} from "../../errors/index.js"; | ||||||
import type { NetworkError } from "../../errors/index.js"; | ||||||
import { Observable } from "../../utilities/index.js"; | ||||||
import type { Operation, FetchResult, NextLink } from "../core/index.js"; | ||||||
|
@@ -8,6 +16,10 @@ import { ApolloLink } from "../core/index.js"; | |||||
export interface ErrorResponse { | ||||||
graphQLErrors?: ReadonlyArray<GraphQLFormattedError>; | ||||||
networkError?: NetworkError; | ||||||
protocolErrors?: ReadonlyArray<{ | ||||||
message: string; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At this point, these errors become confusing without explanation or context.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know we call those "protocol errors" internally, but looking at when these actually occur (only on subsequent chunks of http multipart requests), could we maybe call this
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I named this These errors aren't actually incremental errors but rather the transport-level errors sent from the router that occur when it can't communicate with the subgraph (i.e. the |
||||||
extensions?: GraphQLErrorExtensions[]; | ||||||
}>; | ||||||
response?: FormattedExecutionResult; | ||||||
Comment on lines
+31
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could also just be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I used the type defined in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, maybe it might also make sense to just reference |
||||||
operation: Operation; | ||||||
forward: NextLink; | ||||||
|
@@ -42,16 +54,24 @@ export function onError(errorHandler: ErrorHandler): ApolloLink { | |||||
operation, | ||||||
forward, | ||||||
}); | ||||||
} else if (graphQLResultHasProtocolErrors(result)) { | ||||||
retriedResult = errorHandler({ | ||||||
protocolErrors: result.extensions[PROTOCOL_ERRORS_SYMBOL], | ||||||
response: result, | ||||||
operation, | ||||||
forward, | ||||||
}); | ||||||
} | ||||||
|
||||||
if (retriedResult) { | ||||||
retriedSub = retriedResult.subscribe({ | ||||||
next: observer.next.bind(observer), | ||||||
error: observer.error.bind(observer), | ||||||
complete: observer.complete.bind(observer), | ||||||
}); | ||||||
return; | ||||||
} | ||||||
if (retriedResult) { | ||||||
retriedSub = retriedResult.subscribe({ | ||||||
next: observer.next.bind(observer), | ||||||
error: observer.error.bind(observer), | ||||||
complete: observer.complete.bind(observer), | ||||||
}); | ||||||
return; | ||||||
} | ||||||
|
||||||
observer.next(result); | ||||||
}, | ||||||
error: (networkError) => { | ||||||
|
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 makes a lot of assumptions about what the HttpLink will do. I'll see if I can rewrite the test to use an actual HttpLink