32561 POC: Decode error objects into user-friendly messages (DO NOT MERGE)#841
32561 POC: Decode error objects into user-friendly messages (DO NOT MERGE)#841severinbeauvais wants to merge 1 commit intobcgov:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new error-decoding mixin and wires it into the app’s save-error handling so that HTTP 400/422 API error objects can be converted into more user-friendly strings for display in the Save Error dialog.
Changes:
- Added
ErrorMessageMixinwith a path-based lookup table and adecodeErrorMessage()helper. - Updated
App.vueto use the new mixin and decode errors for HTTP 400/422 before showing the save error dialog. - Updated Save Error dialog copy to instruct users to correct data and resubmit.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/mixins/index.ts |
Exports the new ErrorMessageMixin for use across the app. |
src/mixins/error-message-mixin.ts |
Implements path-to-message mapping and error decoding logic. |
src/dialogs/SaveErrorDialog.vue |
Updates the error dialog text shown to users. |
src/App.vue |
Integrates the mixin and decodes 400/422 errors for display in the dialog. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // on HTTP 400 or 422, decode error messages for display in dialog | ||
| if ([StatusCodes.BAD_REQUEST, StatusCodes.UNPROCESSABLE_ENTITY].includes(error?.response?.status)) { | ||
| this.saveErrors.forEach(err => { | ||
| err.error = this.decodeErrorMessage(err, filing) | ||
| }) | ||
| } |
There was a problem hiding this comment.
New behavior is introduced here (decoding 400/422 API errors into user-friendly strings) but there are no unit tests covering it. Since the repo already has comprehensive unit tests for App/dialog error flows, consider adding tests that exercise the save-error-event handler for both 400 and 422 responses (including schema errors with context[].jsonPath and non-schema errors with path) and assert that the dialog receives the decoded messages.
| static getByPath (path: string): any { | ||
| return path | ||
| .replace(/\[(\w+)\]/g, '.$1') // convert [0] → .0 | ||
| .split('.') | ||
| .reduce((o, key) => (o ? o[key] : undefined), ErrorMessageMixin.errorMessages) | ||
| } |
There was a problem hiding this comment.
getByPath() assumes path is always a non-empty string and immediately calls replace()/split(). However decodeErrorMessage() can pass undefined when err.context[0].jsonPath or err.path is missing, which will throw at runtime and prevent the save error dialog from rendering. Consider making getByPath(path?: string) return undefined when path is falsy and/or adding a fallback path string in decodeErrorMessage() when the error object lacks a path.
There was a problem hiding this comment.
Yes, this needs to be fixed in a future commit.
| this.saveErrors = error?.response?.data?.rootCause?.errors || [] | ||
| this.saveWarnings = error?.response?.data?.rootCause?.warnings || [] | ||
| const filing = error?.response?.data?.rootCause?.filing || {} |
There was a problem hiding this comment.
The save error handler now reads errors/warnings only from error.response.data.rootCause. If the backend (or an upstream gateway) returns the previous shape (error.response.data.errors / warnings) this will result in an empty saveErrors array and the dialog will fall back to a generic message. To avoid a regression, consider supporting both shapes (eg, use rootCause when present, otherwise fall back to data.errors/data.warnings).
There was a problem hiding this comment.
If the API GW is changed and the error reponse changes, this isn't the only place to fix, so that can be a global exercise and I'd prefer to leave this code as-is for now.
severinbeauvais
force-pushed
the
32561
branch
from
8e9502e to
b291f83
Compare
| this.setHaveChanges(false) | ||
| this.paymentErrorDialog = true | ||
| } else { | ||
| console.log('Save error =', error) // eslint-disable-line no-console |
There was a problem hiding this comment.
The error was already logged previously; there's no need for a second log.
| isFutureEffective: 'Error in header object', | ||
| name: 'Error in header object', | ||
| priority: 'Error in header object', | ||
| waiveFees: 'Error in header object' |
There was a problem hiding this comment.
This duplication could probably be simplified if we use a different mapping algorithm.
A different mapping algorithm could also handle the "nth" element of various arrays (and maybe even include special handling to incorporation the name of the party/share class/series).
severinbeauvais
changed the title
- small tweak to save error dialog text - created mixin with error messages structure and utlities to decode error objects into user-friendly strings
severinbeauvais
force-pushed
the
32561
branch
from
b291f83 to
39dd103
Compare
| * FUTURE: identify party by name (from filing) | ||
| * FUTURE: identify share class/series by name (from filing) | ||
| */ | ||
| static errorMessages = { |
There was a problem hiding this comment.
@janisrogers Here's the structure to map the "path" to the error message.
LMK what to update :)
Issue #: bcgov/entity#32561
Description of changes:
NOTE: This proof of concept is only implemented for Incorporation Applications thus far.
NOTE: To test, you need some bad data. Since the UI won't let you enter bad data, you can modify the filing JSON in the db then resume the filing and try to submit it with this code. (I corrupted the data locally while developing.)
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the bcrs-entities-create-ui license (Apache 2.0).