Fix error handling in fetchDocById helper
#867
Merged
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
|
Visit the preview URL for this PR (updated for commit 1522135): https://roar-staging--pr867-ref-318-query-compos-t79uux7m.web.app (expires Thu, 17 Oct 2024 22:02:25 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 2631e9c58fd0104ecbfddd72a62245ddac467460 |
roar-dashboard-e2e
|
||||||||||||||||||||||||
| Project |
roar-dashboard-e2e
|
| Run status |
|
| Run duration | 02m 01s |
| Commit |
|
| Committer | Maximilian Oertel |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
17
|
ksmontville
reviewed
Oct 10, 2024
| try { | ||
| const activationCode = await fetchDocById('activationCodes', studentCode, undefined, 'admin', true, true); | ||
|
|
||
| if (activationCode.orgId) { | ||
| state.students[outerIndex].orgName = `${_capitalize(activationCode.orgType)} - ${ |
There was a problem hiding this comment.
Can we guarantee that these properties of activationCode i.e. activationCode.orgType exist? Should we add fallbacks or optional chaining?
There was a problem hiding this comment.
I didn't add optional chaining here so that if activationCode.orgType doesn't exist, an error is thrown and caught by the catch block. There is definitely big room for improvement on the error handling, but, in my opinion, that should be done in a separate PR?
Co-authored-by: Kyle Montville <[email protected]>
Co-authored-by: Kyle Montville <[email protected]>
This reverts commit 4fa5c46.
maximilianoertel
requested review from
ksmontville,
richford,
Emily-ejag and
kellyel
…mposables-fetch-error
maximilianoertel
changed the base branch from
ref/318/query-composables
to
ref/318/query-composables-update
richford
approved these changes
Oct 11, 2024
Base automatically changed from
ref/318/query-composables-update
to
ref/318/query-composables
October 11, 2024 15:48
21 tasks
maximilianoertel
added
bug
Emily-ejag
approved these changes
Oct 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Proposed changes
This PR removes the
swallowErrorsparameter on thefetchByDocIdhelper to, well, prevent errors from being swallowed. Doing so and allowing the helper to throw any errors, enables any TanStack query using this helper function to properly handle error cases and trigger automatic retries.As this parameter was only used in the
validateCodemethod, this PR also slightly refactors that function to handle thrown errors.Types of changes
Checklist
Justification of missing checklist items
n/a
Further comments
n/a
Ref https://github.com/yeatmanlab/roar/issues/318