Skip to content
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

Fix failure to return function reference #693

Merged
merged 2 commits into from
Sep 24, 2019
Merged

Conversation

sabrehagen
Copy link
Contributor

What does this PR do?

Resolves a bug

Motivation

When wrapping my middleware chain with tracer.wrap I found that errors thrown in downstream middleware were not caught by the upstream error handler. When disabling tracer.wrap, the error handler caught the rejected promise as expected.

Changes

Changed the code to return the reference to fn.apply so the return value (e.g. a promise) is not lost.

@sabrehagen sabrehagen requested a review from a team as a code owner September 24, 2019 01:54
Copy link
Member

@rochdev rochdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Can you add a test to avoid future regressions?

@rochdev rochdev added bug Something isn't working community core labels Sep 24, 2019
@sabrehagen
Copy link
Contributor Author

Test added as requested 👍 .

@rochdev
Copy link
Member

rochdev commented Sep 24, 2019

Failing tests are also failing on the nightly build and are unrelated to this PR.

@rochdev rochdev merged commit 3788520 into DataDog:master Sep 24, 2019
@sabrehagen
Copy link
Contributor Author

Could you please make a release so we can consume this fix :)

rochdev pushed a commit that referenced this pull request Sep 26, 2019
* Fix failure to return function reference

* add-test
@rochdev
Copy link
Member

rochdev commented Sep 26, 2019

Released in 0.15.3.

@rochdev rochdev added this to the 0.15.3 milestone Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working community core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants