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

Dispatch errors from the primary response to deferred responses #2192

Merged
merged 34 commits into from
Dec 12, 2022

Conversation

Geal
Copy link
Contributor

@Geal Geal commented Dec 1, 2022

Fix #1818
Fix #2185
Blocked by #2109 (waiting for the router-bridge update)

When errors are generated during the primary execution, some of them can be affected to
deferred responses.

To handle that, we need to:

  • transmit errors from the primary query to deferred node execution along with the primary fetches
  • be able to check if an error path belongs to a deferred query

Since the error path may belong to a part of the response that was nullified, we need to follow the error path through the primary or deferred queries

@github-actions

This comment has been minimized.

@Geal Geal force-pushed the geal/dispatch-errors-from-primary branch 2 times, most recently from 456e91d to 45f839c Compare December 6, 2022 14:27
@Geal Geal changed the title Geal/dispatch errors from primary Dispatch errors from the primary response to deferred responses Dec 6, 2022
an error path might affect a part of the response that is not available
anymore. We have to check if that error belongs to the primary query or
any of the deferred queries to dispatch the error to the corresponding
response
@Geal Geal force-pushed the geal/dispatch-errors-from-primary branch from 2c6d4e5 to d564e36 Compare December 12, 2022 10:27
@Geal Geal changed the base branch from dev to geal/split-queries December 12, 2022 10:27
@@ -23,24 +23,6 @@ expression: stream.next_response().await.unwrap()
"suborga",
0
]
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is removing the redundant parts of the snapshot, now that error dispatching is fixed

NEXT_CHANGELOG.md Outdated Show resolved Hide resolved
Base automatically changed from geal/split-queries to dev December 12, 2022 11:17
@Geal Geal marked this pull request as ready for review December 12, 2022 12:01
apollo-router/src/services/execution_service.rs Outdated Show resolved Hide resolved
apollo-router/src/services/execution_service.rs Outdated Show resolved Hide resolved
Co-authored-by: Jeremy Lempereur <jeremy.lempereur@iomentum.com>
@Geal Geal enabled auto-merge (squash) December 12, 2022 12:09
@Geal Geal self-assigned this Dec 12, 2022
@Geal Geal disabled auto-merge December 12, 2022 16:16
@Geal Geal enabled auto-merge (squash) December 12, 2022 16:16
@Geal Geal force-pushed the geal/dispatch-errors-from-primary branch from f2cc4e3 to 60a3b2e Compare December 12, 2022 16:32
@Geal Geal merged commit 20cceda into dev Dec 12, 2022
@Geal Geal deleted the geal/dispatch-errors-from-primary branch December 12, 2022 16:49
@BrynCooke BrynCooke added this to the v1.6.0 milestone Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants