Preserve plain-object rejection reasons in global error handlers#181
Preserve plain-object rejection reasons in global error handlers#181
Conversation
f3c071b to
f450310
Compare
| * @param value - Any already-safe value prepared by the caller | ||
| * @returns The error name string, or undefined if absent or empty | ||
| */ | ||
| export function getTypeFromError(value: unknown): HawkJavaScriptEvent['type'] | undefined { |
There was a problem hiding this comment.
it will be simpler
| * @param context - any additional data passed by user | ||
| */ | ||
| private async prepareErrorFormatted(error: Error | string, context?: EventContext): Promise<CatcherMessage<typeof Catcher.type>> { | ||
| private async prepareErrorFormatted(error: ErrorSource, context?: EventContext): Promise<CatcherMessage<typeof Catcher.type>> { |
There was a problem hiding this comment.
| private async prepareErrorFormatted(error: ErrorSource, context?: EventContext): Promise<CatcherMessage<typeof Catcher.type>> { | |
| private async prepareErrorFormatted(errorOrRejectionReasion: ErrorSource, context?: EventContext): Promise<CatcherMessage<typeof Catcher.type>> { |
| fallbackTitle: event.message | ||
| ? (event.filename ? `'${event.message}' at ${event.filename}:${event.lineno}:${event.colno}` : event.message) | ||
| : undefined, | ||
| }; | ||
| } | ||
|
|
||
| if (event.type === 'unhandledrejection') { | ||
| event = event as PromiseRejectionEvent; | ||
|
|
||
| return { | ||
| rawError: event.reason, | ||
| fallbackType: 'UnhandledRejection', |
There was a problem hiding this comment.
specifying of fallback title and type is a part of internal business logic of getErrorFromErrorEvent. It should not be exported
| event = event as PromiseRejectionEvent; | ||
|
|
||
| return { | ||
| rawError: event.reason, |
| if (event instanceof ErrorEvent && error === undefined) { | ||
| error = (event as ErrorEvent).message; | ||
| } | ||
| const error = getErrorFromErrorEvent(event); |
There was a problem hiding this comment.
maybe it should return only type, title, backtrace?
const { type, title, backtrace } = getErrorDetailsFromEvent(event);| stack: event.error?.stack || '', | ||
| type, | ||
| message, | ||
| stack: (errorSource.rawError as Error)?.stack || '', |
There was a problem hiding this comment.
rawError is typed as unknown, so casting to Error before accessing .stack is unsafe — the cast bypasses the type system without any runtime guarantee. The optional chain prevents a throw but silently produces undefined for non-Error values.
Prefer an explicit guard (same fix applies on line 222):
stack: errorSource.rawError instanceof Error ? errorSource.rawError.stack ?? '' : '',| @@ -212,8 +218,8 @@ export class ConsoleCatcher { | |||
| method: 'error', | |||
| timestamp: new Date(), | |||
| type: 'UnhandledRejection', | |||
There was a problem hiding this comment.
Shouldn't we use here const type from 202 line?
| */ | ||
| export function getErrorFromErrorEvent(event: ErrorEvent | PromiseRejectionEvent): ErrorSource { | ||
| if (event.type === 'error') { | ||
| event = event as ErrorEvent; |
There was a problem hiding this comment.
I know that it's no-op, but event = event as ErrorEvent reads ike mutation. Perhaps it'll be better to bind a local const:
const errorEvent = event as ErrorEvent;
return {
rawError: errorEvent.error,
fallbackTitle: ...,
};Same for unhandledrejection branch.
| const sanitizedError = Sanitizer.sanitize(rawError); | ||
| const throwableError = rawError instanceof Error ? rawError : undefined; | ||
| const title = getTitleFromError(sanitizedError) ?? fallbackTitle ?? '<unknown error>'; | ||
| const type = getTypeFromError(sanitizedError) ?? fallbackType; |
There was a problem hiding this comment.
When rawError is an Error (e.g. Promise.reject(new TypeError('...'))), getTypeFromError returns 'TypeError' and fallbackType: 'UnhandledRejection' is silently discarded.
Previously, type was always the error's own name for Error objects – so this is not a regression – but it's a non-obvious, especially since the console catcher diverges here (see consoleCatcher.ts:220).
Perhaps we should leave comment explaining current behavior.
|
|
||
| it('should return undefined when name is absent', () => { | ||
| expect(getTypeFromError(Sanitizer.sanitize({ code: 'ERR_001' }))).toBeUndefined(); | ||
| }); |
There was a problem hiding this comment.
Two cases worth adding to this describe block:
- Sanitized DOMException —
Sanitizer.sanitizeconverts it to a string placeholder ('<instance of DOMException>'), sogetTypeFromErrorreturnsundefined. ThegetTitleFromErrorsuite already covers this input, butgetTypeFromErrordoesn't:
it('should return undefined for sanitized DOMException', () => {
expect(getTypeFromError(Sanitizer.sanitize(new DOMException('Network error', 'NetworkError')))).toBeUndefined();
});- Plain object with a
namefield — some libraries reject with{ name, message }shapes; this is the path that returns a non-undefined type from a plain object:
it('should return name from plain object with name field', () => {
expect(getTypeFromError(Sanitizer.sanitize({ name: 'CustomError', code: 42 }))).toBe('CustomError');
});
Closes #171
Fix global error normalization so plain-object promise rejection reasons are captured as readable payloads instead of "[object Object]".
PR Description
Fix error normalization for global error handlers so rejected plain objects are preserved instead of being collapsed into
"[object Object]".This PR introduces a normalized
CapturedErrorshape and uses it across the main catcher and console catcher. Forunhandledrejection, plain-object reasons are now serialized into a readable title instead of going throughString(obj). For browserErrorEvents, the catcher now also builds a more useful fallback message when the original error object is unavailable (for example, cross-origin script errors).Changes
fillCapturedErrorandgetErrorFromErrorEventhelpers insrc/utils/error.tsCatcherinternals fromError | stringto normalizedCapturedErrorconsoleCatcherrawErrorreference for deduplication/backtrace logicErrorDOMExceptionErrorEventfallback