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

MockLink tweaks to avoid uncaught exceptions #7110

Merged
merged 6 commits into from
Oct 10, 2020
Merged

Conversation

hwillson
Copy link
Member

@hwillson hwillson commented Oct 2, 2020

Ensure MockLink (used by MockedProvider) returns all errors through the Link's Observable, instead of throwing some errors (like No more mocked responses for the query ...) leading to uncaught exceptions. All errors are now available via the error property of a result, and MockLink will no longer trigger any uncaught exceptions.

Fixes #6559

Please note: Returning all errors through the Link's Observable was the default behavior in Apollo Client 2.x. We changed it for 3, but the change has been problematic for those looking to migrate from 2.x to 3. We've decided to change this back with the understanding that not many people want or are relying on MockLink's throwing exception approach. If anyone is relying on this functionality, it can be re-created by leveraging MockLink.setOnError as follows:

const mocks = // ...your mocks...
const mockLink = new MockLink(mocks);
mockLink.setOnError(error => { throw error; });
// ... then call with something like
<MockedProvider link={mockLink}>

src/__tests__/fetchMore.ts Outdated Show resolved Hide resolved
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

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

@hwillson I think I see a way to make this less of a breaking change for people who already have code that uses setOnError. Let me know what you think!

src/utilities/testing/mocking/mockLink.ts Outdated Show resolved Hide resolved
src/__tests__/fetchMore.ts Outdated Show resolved Hide resolved
This means errors are returned through a Link's Observable by
default, instead of thrown as an uncaught exception.
Instead of testing errors as uncaught exceptions.
Now that errors are back to being handled in the link chain, some
of the custom setOnError functions we added are rejecting tests
before we have a chance to validate the expected errors. This
commit removes the conflicting setOnError functions.
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

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

One final question: if we're passing errors thrown by onError to observer.error, I'm not sure the guidance we're giving in the CHANGELOG.md about setting mockLink.setOnError(error => { throw error }) will work, since that thrown error will just be passed to observer.error. I think that's a fine/good behavior, but perhaps we could simplify the recommendation to say setOnError can be used to customize the error handling?

@hwillson
Copy link
Member Author

CHANGELOG updated @benjamn, so I think we're all set here. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants