Error handling#95
Open
AlexSurtees wants to merge 7 commits into
Open
Conversation
✅ Deploy Preview for biongff-vizarr ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Merged
There was a problem hiding this comment.
Pull request overview
This PR introduces a new error/warning handling pathway for the viewer by moving Zarr opening into a dedicated HTTP service layer, adding a consumer-provided logger, and surfacing metadata-derived warnings via snackbars; it also adds a new Vitest suite intended to cover common HTTP/metadata failures.
Changes:
- Add
viewer/src/services/http.tswith custom error classes andopenZarrRoot()to provide more structured HTTP/metadata failure handling. - Add warning detection (
getSourceDataWarnings) and notistack-based warning display inVizarrViewer, plus expose aloggerprop for consumers. - Add a new
viewer/tests/error-handling.test.tsand local fixture scaffolding for error-condition testing.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| viewer/tests/metadata.js | Updates test utility to use openZarrRoot() instead of removed utils.open(). |
| viewer/tests/error-handling.test.ts | Adds new test suite + local HTTP server for validating common HTTP/metadata error handling. |
| viewer/src/utils.ts | Removes utils.open() helper and keeps store normalization unchanged functionally. |
| viewer/src/state.ts | Removes sourceWarningAtom (warnings no longer stored in global state). |
| viewer/src/services/http.ts | New HTTP/Zarr open wrapper with custom error types and special-casing. |
| viewer/src/io.ts | Switches source opening to openZarrRoot() and tweaks an assertion string formatting. |
| viewer/src/error.ts | Simplifies user error message behavior; adds warning detection + centralized error side effects. |
| viewer/src/components/VizarrViewer.tsx | Integrates logger prop, warning snackbars, and new error handling flow. |
| viewer/src/components/Snackbar.tsx | Removes old snackbar component implementation. |
| viewer/src/components/LayerController/ChannelOptions.tsx | Removes channel-name warning emission from per-channel UI. |
| viewer/src/api.tsx | Introduces Logger interface type for the viewer API surface. |
| sites/app/src/App.tsx | Adds a console-backed logger and passes it into the Viewer component. |
| fixtures/local/only_zarr_json.zarr/zarr.json | Adds minimal local fixture intended for metadata edge-case testing. |
| fixtures/generic/c4904df5-0005-4a80-953e-62b1ede447b4.yaml | Removes an unused generic fixture entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+60
to
+66
| if (status === 403) { | ||
| throw new HttpError( | ||
| `403: Unauthorized to access resource at ${source}. Please check the specified URL is correct and that permission to access it is not restricted.`, | ||
| statusText, | ||
| status, | ||
| ); | ||
| } |
Comment on lines
+80
to
+85
| if (error instanceof TypeError) { | ||
| if (error.message === MAYBE_CORS_ERROR_MESSAGE) { | ||
| throw new HttpError( | ||
| `An unknown error occurred while trying to fetch the resource ${source} from the server - this is most likely a CORs issue.`, | ||
| error.message, | ||
| ); |
Comment on lines
48
to
59
| export interface VizarrViewerProps { | ||
| sources?: string[]; | ||
| viewState?: ViewState; | ||
| onViewStateChange?: (viewState: ViewState) => void; | ||
| onViewerStateChange?: (info: ViewerInfo) => void; | ||
| additionalLayers?: Layer[]; | ||
| pluginCursor?: string; | ||
| onPluginClick?: (coordinate: [number, number]) => boolean; | ||
| onPluginHover?: (coordinate: [number, number] | null) => void; | ||
| children?: React.ReactNode; | ||
| logger: Logger; | ||
| } |
Comment on lines
191
to
195
| if (!sourceDataValid(results)) { | ||
| setSourceError(writeUserErrorMessage(getSourceDataError(results))); | ||
| const error = getSourceDataError(results); | ||
| setSourceError(writeUserErrorMessage(error)); | ||
| handleError(error, logger); | ||
| } |
Comment on lines
+199
to
+202
| const warnings = getSourceDataWarnings(res.value); | ||
| warnings.map((warning: string) => { | ||
| handleWarning(warning); | ||
| }); |
| } | ||
| } | ||
|
|
||
| export async function openZarrRoot(source: string | zarr.Readable): Promise<zarr.Group<zarr.Readable<unknown>>> { |
Comment on lines
+76
to
+78
| const store = await normalizeStore(source); | ||
| const location = await zarr.open(store, { kind: "group" }); | ||
| return location; |
| server.close(); | ||
| }); | ||
|
|
||
| beforeEach(() => { |
Comment on lines
+63
to
+66
| server.listen(() => {}); | ||
| port = server.address().port; | ||
| server_url = `http://localhost:${port}`; | ||
| }); |
Collaborator
Author
|
@davehorsfall, conflicts resolved, waiting for review/merge. |
This was referenced Jul 7, 2026
This file contains hidden or 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
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.
Description
Overview of architecture/design decisions
Errors thrown from within the createSourceData function, which encompasses the majority of the internal logic of the viewer, are handled: taking the source URL and creating a source data object with all of the information necessary for deck.gl to render it. Errors occurring during the rendering of the image, e.g. from deck.gl still need to be handled.
createSourceData returns a promise, any errors raised during execution of the function are returned and then handled via the error.ts module. Currently, errors are thrown and logged. The VizarrViewer component renders the message. Where possible, custom error handlers are used to catch common errors and produce user-friendly error messages, and react-component-consumer-friendly error classes which can be used for downstream error handling (e.g. if instanceof MetadataError...)
If there are no errors, the source data is analysed for any potential warnings. These warnings are then displayed as stackable, dismiss-able toast notifications. Warnings generated in this way are also logged.
Warnings can be tested here
https://deploy-preview-95--biongff-vizarr.netlify.app/?source=https://s3.janelia.org/ahrens-lab/ruettenv/ExM/250531_foxj1a_eGFP_NPVF_mCherry_7dpf/dorsal/250531_foxj1a_egfp_npvf_mcherry_7dpf_dorsal.zarr
The design of the error handling/warning system allows plenty of flexibility in how warnings and errors are handled. The raising of errors and warnings is decoupled from the handling. Any additional side effects that need to be implemented when errors or warnings are raised can easily be added, e.g. via the handleError() function in the error.ts module.
Further work will:
Fixes # (issue)
#29
#46
#19
#78
#89
Type of change