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(core): undo prior manual event detection #2287

Merged
merged 3 commits into from
Feb 17, 2022
Merged

Conversation

JoviDeCroock
Copy link
Collaborator

@JoviDeCroock JoviDeCroock commented Feb 17, 2022

Summary

In #2210 I've done an attempt at catching errors that happen in the backwards chain of the exchange-stream. This because we have this part of the stream forked since #1854 to support incremental fetch-results.

The downside of this is that the executeIncrementalFetch is wrapped by a .catch itself to account for erroneous results from the server. In this attempt we tried to catch both TypeError as well as SyntaxError as those are the most common ones. However, failed to fetch is an example of a TypeError that actually comes from the fetch call which we want to report to the client.

I'm not entirely sure how we can prevent swallowing errors, one potential avenue would be to wrap user-supplied functions and mark them as errored out in the exchanges but this is a big caveat to have towards external exchange-authors. Trying to think of a way we could make the response-stream report errors during dev, maybe even just a console.error? However that could prove problematic for production systems that want to report this...

EDIT: I've added one possible solution that moves the closure of executeIncrementalFetch within the fetchSource this way we can effectively track whether we are on the last result, the only case I'm in doubt about would be failed to fetch

Set of changes

@changeset-bot
Copy link

changeset-bot bot commented Feb 17, 2022

🦋 Changeset detected

Latest commit: 9b4be79

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@urql/core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@JoviDeCroock JoviDeCroock changed the title undo prior art of catching errors manually fix(core): undo prior manual event detection Feb 17, 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
2 participants