-
Notifications
You must be signed in to change notification settings - Fork 39
fix: handle the side-effect artifacts case #683
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
Conversation
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.
This change might need to also be checked if needed in dd-trace-js integration
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.
where in dd-trace-js?
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 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.
That patching is safe. It's because it is yet another layer of wrapping above the patching done here in this repo.
My understanding is that this promisifiedHandler function exists in order to make it easier for the wrapping to add pre and post run handling, i.e., enabling it to use try { await the_promisifiedHandler} finally {}. And it just need to be done once. So dd-trace-js part should be good.
What does this PR do?
Handle the case where the original handler code returns a side-effect artifact. Such as
aws-serverless-express server.Motivation
https://datadoghq.atlassian.net/browse/SLES-2468
Testing Guidelines
Additional Notes
Types of Changes
Check all that apply