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

Log non-fatal error when fields are missing from written results #8416

Merged
merged 4 commits into from
Jun 22, 2021

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Jun 22, 2021

Fixes #8331 and #6915 by relaxing an overzealous exception to be merely an invariant.error message, which should also help with the underlying cause of #7436 (comment).

Although this change is likely to trigger a number of new missing field error messages for existing applications, these errors are almost always worth investigating, because they can hide gaps in the incoming data that manifest themselves as missing fields later, when the original cause of the error is much less obvious.

Because this error was an exception before, we had to be very careful about when we threw it, since it would abort the entire write. Unfortunately, this carefulness led to some dodgy decisions about when to hide exceptions that really should have been surfaced (see #8331 for one example, where merely using possibleTypes would enable the exception). Now that we're just logging an error message using invariant.error, those messages can be a little noisier without disrupting the overall work of the client/cache.

In production, invariant.error messages are stripped out, along with invariant.warn and invariant.info, so these errors should never be visible in production builds (unlike the previous exceptions, which were thrown the same way in both production and development builds).

I hope developers find these error messages useful, though of course we can continue to refine the preconditions/frequency of the messages, and/or ways to capture/redirect errors for certain writes, where the errors are known to be harmless, or need to be programmatically processed in some way.

Comment on lines 2393 to +2396
const [mutate, { loading: mutationLoading }] = useMutation(mutation, {
optimisticResponse: carData,
optimisticResponse: {
addCar: carData,
},
Copy link
Member Author

Choose a reason for hiding this comment

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

One of the most common mistakes surfaced by these errors is providing only the mutation field's value (carData), rather than the field itself ({ addCar: carData }), when providing an optimisticResponse.

@benjamn benjamn force-pushed the relax-writeToStore-missing-field-exception branch from 8c83617 to 1daffc2 Compare June 22, 2021 18:47
@benjamn benjamn merged commit 9efc407 into release-3.4 Jun 22, 2021
@benjamn benjamn deleted the relax-writeToStore-missing-field-exception branch June 22, 2021 18:50
@hwillson hwillson removed this from the MM-2021-06 milestone Jul 29, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
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.

3 participants