Enhance DOI Search Error Handling #450
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
…rrors when API responses are not OK
akita
|
||||||||||||||||||||||||||||
| Project |
akita
|
| Branch Review |
issue-2286
|
| Run status |
|
| Run duration | 01m 50s |
| Commit |
|
| Committer | Joseph Rhoads |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
3
|
Skipped
|
0
|
Passing
|
50
|
| View all changes introduced in this branch ↗︎ | |
…search and works listing
…fy error and empty state handling
…rror and empty state handling
…combine error and empty state handling
…pecific entity-based messages
bklaing2
approved these changes
Feb 4, 2025
| @@ -23,34 +22,15 @@ export default function SearchOrganizations(props: Props) { | |||
|
|
|||
| if (loading) return <Row><Loading /></Row> | |||
|
|
|||
| if (error) return ( | |||
| const organizations = data?.organizations | |||
| if (error || !organizations || organizations.nodes.length == 0) return ( | |||
There was a problem hiding this comment.
Is there any reason we may want to keep the granularity of splitting the error out from no results and showing the error component? @jrhoads
There was a problem hiding this comment.
If we ever want to in the future, we can split this up again.
| options | ||
| ) | ||
| const json = await res.json() | ||
| if(!res.ok) { |
There was a problem hiding this comment.
Do we want to do the error handling this way on all the REST fetches? @jrhoads
There was a problem hiding this comment.
Yeah. We had been catching the error. React/Tanstack query expects an error to be thrown. Otherwise it never populates the error state.
codycooperross
approved these changes
Feb 4, 2025
akita
|
||||||||||||||||||||||||||||
| Project |
akita
|
| Branch Review |
1.18.1
|
| Run status |
|
| Run duration | 02m 00s |
| Commit |
|
| Committer | Joseph Rhoads |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
Failures
|
0
|
|
Flaky
|
0
|
|
Pending
|
3
|
|
Skipped
|
0
|
|
Passing
|
50
|
| View all changes introduced in this branch ↗︎ | |
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.
Purpose
Improve error handling and response processing in DOI search and facets queries to provide more informative error messages and ensure proper error propagation.
Approach
The changes enhance the error handling mechanism for DOI search and facets API requests by:
Key Modifications
fetchDoisandfetchDoisFacetsfunctionsImportant Technical Details
NoResultscomponent that can be used across different search typesTypes of changes
The changes improve error handling by providing more transparent and informative error messages during DOI search API requests, making debugging and error tracking more straightforward.
Reviewer, please remember our guidelines: