Refactor Log and Error Types and Handling#1933
Draft
AaronPlave wants to merge 6 commits into
Draft
Conversation
Single writable + two helpers (catchError, logMessage) that take a category first arg. Per-tab readables (schedulingErrors, simulationDatasetErrors, constraintRunErrors, allLogs, errorLogs, allProblems) are derived filters and keep their original public names. Deletes SchedulingError, SimulationDatasetError, ConstraintRunError in favor of LogMessage with an optional category. Routes thrown exceptions during schedule/simulate/checkConstraints to the matching tab rather than the generic Logs tab — UI half of NASA-AMMOS/plandev#1777.
The unified store holds backend exceptions, thrown errors, info logs, and validation problems through a single shape. "BaseError" and the "errors.ts" filenames implied error-only content; renaming makes the types and file names match what the code actually represents. - BaseError → ConsoleEntry across all consumers and component props. - src/types/errors.ts → src/types/console.ts. - src/stores/errors.ts → src/stores/console.ts (and .test.ts). - Per-tab readables: simulationDatasetErrors → simulationErrors, constraintRunErrors → constraintErrors (the "Dataset" / "Run" qualifiers referenced backend entities that no longer have a 1:1 relationship with the store contents). src/utilities/errors.ts kept its name — it genuinely holds error domain code (ErrorTypes enum, validation type guards).
reqWorkspace and reqWorkspaceMetadata were discarding non-OK response bodies and throwing a plain Error(statusText), losing the full backend exception payload (type, service, data, trace, cause) that PR #1777 emits via WorkspaceFormattedError. Workspace endpoints use USE_HASURA_FORMATTING=false, so the body is the raw FormattedError JSON shape rather than the Hasura {message, extensions: {...}} wrapper. Parses it directly into a ConsoleEntry and throws a CompoundError so workspace effect callers (which already use catchError(...)) get typed routing into the Workspace Console's Logs tab. Falls back to throw new Error(response.statusText) when the body isn't JSON or doesn't have the FormattedError shape. Adds 3 unit tests in src/utilities/requests.test.ts covering the happy path, non-FormattedError body, and non-JSON body.
The bulk move/delete/upload handlers were collapsing per-item
FormattedError responses into a single JS Error with a JSON-
stringified `cause` of "<item>:<cause-string>". That dropped each
item's backend `type`, `service`, `data`, and Java `trace` — leaving
the Workspace Console's Logs tab with only a generic message and a
JS stack trace.
Adds `buildBulkOperationCompoundError(failedOps, verb)` in
utilities/workspaces.ts: maps each failed item's FormattedError
response (already LogMessage-shaped) into a CompoundError with one
LogMessage per item. Each becomes its own Console entry with full
backend metadata when expanded.
Also removes the inner try/catch re-wrap in moveWorkspaceItems that
produced the doubled "Unable to move workspace item: Workspace file
unable to be moved" message — and the re-wrap was using the Error
constructor incorrectly (passing the inner Error as the second arg
instead of `{ cause: e }`).
…ccesses Two refinements on top of the bulk-failure CompoundError fix: 1. buildBulkOperationCompoundError no longer prepends "<item>: " to the per-item message. The backend's WorkspaceFormattedError messages already include the file path (e.g. "Unable to move '1f11/A' in Workspace 19 to './A' in Workspace 19."), so the prefix produced a duplicate. The item is now stashed into the entry's `data.item` field instead, surfaced in the expanded view for cases where the backend message omits the path. 2. bulkMoveWorkspaceItems now logs an INFO entry for each item that DID succeed when there are partial failures. Uses the backend's success response string directly (e.g. "'1f11/C' in Workspace 19 moved to './C' in Workspace 19"). Previously the helper threw immediately on any failure, so partial-success runs only surfaced the failures.
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.
___REQUIRES_AERIE_PR___="1777"Backend PR NASA-AMMOS/plandev#1777 unifies every Java exception response to one shape under
errors[].extensions. @Mythicaeda identified two UI issues in her testing:extensions.type— every backend error was relabeled as the genericCAUGHT_ERROR.This PR fixes both and additionally refactors/consolidates the error/log pipeline. Previously the pipeline used three different typed error interfaces, three stores, and three catch helpers to mirror the backend's three-different-response-shape model. Now that the backend returns one shape, the UI can mirror this with a single store and two helpers (catchError and logMessage). Additionally, this PR does a rename pass to change
BaseError->ConsoleEntryanderrors.tsstore toconsole.tsto better align with the realities of what they actually represent now.Visible UX Changes
As a result of this PR, exceptions that occur from Scheduling, Simulation, etc. will appear in their respective console tabs instead of the generic
Logstab.Changes
Unified pipeline — one writable store
consoleEntries: Writable<ConsoleEntry[]>+ two helpers replace three writable stores, three catch helpers, and the oldallLogswritable.catchError(category, message, error)dispatches viainstanceof Errorto handle thrown JS errors and backendConsoleEntrypayloads.logMessage(category, message, options?)for info/timing logs. Per-tab readable stores (schedulingErrors,simulationErrors,constraintErrors,modelErrors,allLogs,errorLogs,allProblems) are derived stores filtered by type onconsoleEntries.Parser fix —
reqHasuranow extractsextensions.type, preservescause/traceseparately (was overwritingtracewithcause), andsimulateusesreason.typeinstead of hardcodedCAUGHT_ERROR.reqWorkspaceandreqWorkspaceMetadatanow parse the rawWorkspaceFormattedErrorbody (workspace usesUSE_HASURA_FORMATTING=false, so noextensionswrapper) and throw it as aCompoundError— workspace callers get typed errors with full backend metadata flowing into the Workspace Console's Logs tab.ErrorTypesenum expanded to cover 27 new backend types.Call-site routing —
schedule/simulate/checkConstraintstag their outercatch (e)with their own category and clear stale entries at start. This routing is done so that backend exceptions land in the appropriate tabs (we cannot rely solely on the backend exceptionservicefield as it could beaerie_permissionswhen the user was running a scheduling request).Renames (cosmetic) —
BaseError→ConsoleEntry;src/types/errors.tsandsrc/stores/errors.ts→console.ts;simulationDatasetErrors/constraintRunErrors→simulationErrors/constraintErrors.Verification
Unit Tests
Manual
Run against a backend on refactor/permissions-service-exceptions:
type,service,data,tracefromextensions.Future Work