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

fix: abortion should not cause incremental completion error #4259

Merged
merged 2 commits into from
Oct 27, 2024

Conversation

yaacovCR
Copy link
Contributor

incremental completion errors are indicated if and only if null bubbling has caused a previously sent response to be null

we must instead just give a partial response with errors within the incremental array

The GraphQL Tools implementation of this feature has different behavior where the next call will throw if the operation is aborted, but we have opted to give partial responses, and so we must adjust.

incremental completion errors are indicated if and only if null bubbling has caused a previously sent response to be null

it should instead just give a partial response with errors within the incremental array
@yaacovCR yaacovCR requested a review from a team as a code owner October 27, 2024 11:57
@yaacovCR yaacovCR added the PR: bug fix 🐞 requires increase of "patch" version number label Oct 27, 2024
Copy link

netlify bot commented Oct 27, 2024

Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
🔨 Latest commit 7605e43
🔍 Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/671e2c2489c13a0008c1ce28
😎 Deploy Preview https://deploy-preview-4259--compassionate-pike-271cb3.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

Hi @yaacovCR, I'm @github-actions bot happy to help you with this PR 👋

Supported commands

Please post this commands in separate comments and only one per comment:

  • @github-actions run-benchmark - Run benchmark comparing base and merge commits for this PR
  • @github-actions publish-pr-on-npm - Build package from this PR and publish it on NPM

@yaacovCR
Copy link
Contributor Author

@JoviDeCroock this was my oversight when reviewing, compounded by another mistake I made in that I allowed a discrepancy between the path of the error and the null site in the response, which should only occur in a situation with null-bubbling. This PR also fixes that.

@yaacovCR yaacovCR merged commit 83da23a into main Oct 27, 2024
35 checks passed
@yaacovCR yaacovCR deleted the switch-abort-check-location branch October 27, 2024 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: bug fix 🐞 requires increase of "patch" version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants