-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
benjamn
merged 4 commits into
release-3.4
from
relax-writeToStore-missing-field-exception
Jun 22, 2021
Merged
Log non-fatal error when fields are missing from written results #8416
benjamn
merged 4 commits into
release-3.4
from
relax-writeToStore-missing-field-exception
Jun 22, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
benjamn
commented
Jun 22, 2021
Comment on lines
2393
to
+2396
const [mutate, { loading: mutationLoading }] = useMutation(mutation, { | ||
optimisticResponse: carData, | ||
optimisticResponse: { | ||
addCar: carData, | ||
}, |
There was a problem hiding this comment.
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
.
brainkim
reviewed
Jun 22, 2021
brainkim
approved these changes
Jun 22, 2021
Fixes #8331 and #6915, and should help with the underlying cause of #7436 (comment)
benjamn
force-pushed
the
relax-writeToStore-missing-field-exception
branch
from
June 22, 2021 18:47
8c83617
to
1daffc2
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 usinginvariant.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 withinvariant.warn
andinvariant.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.