-
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?
Conversation
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage 🔗 Commit SHA: 763bee7 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
Bundles Sizes Evolution
🚀 CPU Performance
🧠 Memory Performance
|
|
/to-staging |
|
View all feedbacks in Devflow UI.
Commit 73dedf8cc3 will soon be integrated into staging-38.
Commit 73dedf8cc3 has been merged into staging-38 in merge commit 18489a1875. Check out the triggered pipeline on Gitlab 🦊 If you need to revert this integration, you can use the following command: |
Integrated commit sha: 73dedf8 Co-authored-by: mormubis <adrian.delarosa@datadoghq.com>
73dedf8 to
671d0da
Compare
| const causes = flattenErrorCauses(error, ErrorSource.CUSTOM) | ||
| expect(causes?.length).toBe(1) | ||
| expect(causes?.[0]).toEqual({ | ||
| message: '"string cause"', // JSON stringified |
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?
| }) | ||
| }) | ||
|
|
||
| it('should stop chain after non-Error cause', () => { |
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.
❓ question: that sounds ok to me but I wonder if it is a choice you made or if it is based on spec.
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.
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 cause field from causing any issues.
Motivation
Fixes #3839
Changes
If the cause of an error is non-error, it will retrieve the same information as
addError(error.cause)and add it to the causes.Test instructions
All tests passed.
Checklist