-
Notifications
You must be signed in to change notification settings - Fork 164
🐛 add support for arbitrary Error causes #3860
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
671d0da
f91c1a0
8f881d2
089bed7
245b580
763bee7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -234,16 +234,6 @@ describe('flattenErrorCauses', () => { | |
| expect(errorCauses).toEqual(undefined) | ||
| }) | ||
|
|
||
| it('should return undefined if cause is not of type Error', () => { | ||
| const error = new Error('foo') as ErrorWithCause | ||
| const nestedError = { biz: 'buz', cause: new Error('boo') } as unknown as Error | ||
|
|
||
| error.cause = nestedError | ||
|
|
||
| const errorCauses = flattenErrorCauses(error, ErrorSource.LOGGER) | ||
| expect(errorCauses?.length).toEqual(undefined) | ||
| }) | ||
|
|
||
| it('should use error to extract stack trace', () => { | ||
| const error = new Error('foo') as ErrorWithCause | ||
|
|
||
|
|
@@ -259,6 +249,96 @@ describe('flattenErrorCauses', () => { | |
| const errorCauses = flattenErrorCauses(error, ErrorSource.LOGGER) | ||
| expect(errorCauses?.length).toEqual(10) | ||
| }) | ||
|
|
||
| describe('with non-Error values', () => { | ||
| it('should handle string cause with consistent structure', () => { | ||
| const error = new Error('main') as ErrorWithCause | ||
| error.cause = 'string cause' | ||
|
|
||
| const causes = flattenErrorCauses(error, ErrorSource.CUSTOM) | ||
| expect(causes?.length).toBe(1) | ||
| expect(causes?.[0]).toEqual({ | ||
| message: '"string cause"', // JSON stringified | ||
| source: ErrorSource.CUSTOM, | ||
| type: undefined, | ||
| stack: undefined, | ||
| }) | ||
| }) | ||
|
|
||
| it('should handle object cause with consistent structure', () => { | ||
| const error = new Error('main') as ErrorWithCause | ||
| error.cause = { code: 'ERR_001', details: 'Invalid input' } | ||
|
|
||
| const causes = flattenErrorCauses(error, ErrorSource.CUSTOM) | ||
| expect(causes?.length).toBe(1) | ||
| expect(causes?.[0]).toEqual({ | ||
| message: '{"code":"ERR_001","details":"Invalid input"}', | ||
| source: ErrorSource.CUSTOM, | ||
| type: undefined, | ||
| stack: undefined, | ||
| }) | ||
| }) | ||
|
|
||
| it('should handle number cause with consistent structure', () => { | ||
| const error = new Error('main') as ErrorWithCause | ||
| error.cause = 42 | ||
|
|
||
| const causes = flattenErrorCauses(error, ErrorSource.CUSTOM) | ||
| expect(causes?.length).toBe(1) | ||
| expect(causes?.[0]).toEqual({ | ||
| message: '42', | ||
| source: ErrorSource.CUSTOM, | ||
| type: undefined, | ||
| stack: undefined, | ||
| }) | ||
| }) | ||
|
|
||
| it('should handle mixed Error and non-Error chain', () => { | ||
| const error1 = new Error('first') as ErrorWithCause | ||
| const error2 = new Error('second') as ErrorWithCause | ||
| error1.cause = error2 | ||
| error2.cause = { code: 'ERR_ROOT' } | ||
|
|
||
| const causes = flattenErrorCauses(error1, ErrorSource.CUSTOM) | ||
| expect(causes?.length).toBe(2) | ||
|
|
||
| // First cause: Error with full structure | ||
| expect(causes?.[0].message).toBe('second') | ||
| expect(causes?.[0].type).toBe('Error') | ||
| expect(causes?.[0].stack).toContain('Error') | ||
|
|
||
| // Second cause: Object with normalized structure | ||
| expect(causes?.[1]).toEqual({ | ||
| message: '{"code":"ERR_ROOT"}', | ||
| source: ErrorSource.CUSTOM, | ||
| type: undefined, | ||
| stack: undefined, | ||
| }) | ||
| }) | ||
|
|
||
| it('should stop chain after non-Error cause', () => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❓ question: that sounds ok to me but I wonder if it is a choice you made or if it is based on spec.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are no specifications regarding how to handle complaint objects that are not errors. I made this decision to prevent a plain object with a |
||
| const error = new Error('main') as ErrorWithCause | ||
| error.cause = { value: 'data', cause: new Error('ignored') } | ||
|
|
||
| const causes = flattenErrorCauses(error, ErrorSource.CUSTOM) | ||
| expect(causes?.length).toBe(1) | ||
| // The entire object is captured, nested cause is sanitized | ||
| expect(causes?.[0].message).toContain('"value":"data"') | ||
| expect(causes?.[0].type).toBeUndefined() | ||
| }) | ||
|
|
||
| it('should handle null cause', () => { | ||
| const error = new Error('main') as ErrorWithCause | ||
| error.cause = null | ||
| expect(flattenErrorCauses(error, ErrorSource.CUSTOM)).toBeUndefined() | ||
| }) | ||
|
|
||
| it('should handle undefined cause', () => { | ||
| const error = new Error('main') as ErrorWithCause | ||
| error.cause = undefined | ||
| expect(flattenErrorCauses(error, ErrorSource.CUSTOM)).toBeUndefined() | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| describe('isError', () => { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💬 suggestion: For reporting thrown string we also stringify the message since we add a prefix the quotes act like a separator, ex:
Uncaught "string cause".Not sure the quotes add much value in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't want to affect how they look in the UI. Also, it improves the readability when they are objects.
Thoughts?