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

Add list of modules breaking async continuity #133

Merged
merged 1 commit into from
Dec 21, 2017

Conversation

watson
Copy link
Member

@watson watson commented Dec 15, 2017

Just starting off the list some some well known sinners 😄

Not sure if the phrase "async continuity" makes sense to people, but it was the best I could come up with - feel free to suggest something else

@jkrems
Copy link
Contributor

jkrems commented Dec 15, 2017

Should we link to existing PRs like petkaantonov/bluebird#1472?

@watson
Copy link
Member Author

watson commented Dec 15, 2017

@jkrems good idea - didn't know there were already a PR. I've updated the table with a link to it.

jkrems

This comment was marked as off-topic.

Qard

This comment was marked as off-topic.

@jkrems
Copy link
Contributor

jkrems commented Dec 21, 2017

@watson Good to merge?

@watson watson merged commit e292e53 into nodejs:master Dec 21, 2017
@watson watson deleted the asynchooks branch December 21, 2017 11:25
@p-bakker
Copy link

p-bakker commented Apr 3, 2019

Don't think generic-pool should be on this list: it uses global.Promise by default (so whatever you have installed) and allows to provide the provide the Promise implementation to use through configuration

@ofrobots
Copy link
Contributor

ofrobots commented Apr 3, 2019

It seems that several APMs are monkey-patching generic-pool to propagate context:

From looking at the patches, it seems that at least acquire loses context. If that is still accurate, generic-pool belongs on the list.

@Qard
Copy link
Member

Qard commented Apr 3, 2019

It's mostly because of pre-promise versions of generic-pool, but also because global.Promise could be set to any promise implementation, not just the native one, so it could be one not supported by our instrumentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants