CLSContext now supports continuations with async-await #201

Ghost---Shadow · 2018-03-31T03:33:55Z

continuation-local-storage has issues with async-await so someone made a library called cls-hooked which fixes that problem.

This PR merely changes the dependency from cls to cls-hooked.

Also, added a test case to demonstrate the phenomenon.

Possible duplicate #135

codefromthecrypt

Thanks for beginning this

There are a number of similar issues and perhaps even pull requests. Can you please note them in this issue and ping people from those issues to review this?

codefromthecrypt · 2018-04-03T12:11:47Z

packages/zipkin-context-cls/test/CLSContext.test.js

+
+    async function newP(id) {
+      const {cid1, cid2} = await ctx.letContext(id, () => afn());
+      // console.log(cid1, cid2);


remove the commented out stuff

codefromthecrypt · 2018-04-03T12:12:01Z

packages/zipkin-context-cls/test/.eslintrc

-  }
-}
+  },
+  "parser": "babel-eslint"


is this change necessary?

Yeah, eslint was freaking out.

codefromthecrypt · 2018-04-03T12:12:37Z

package.json

@@ -33,4 +33,4 @@
  "workspaces": [
    "packages/*"
  ]
-}
+}


might want to change your settings to ensure EOL before EOF

Ghost---Shadow · 2018-04-03T13:18:28Z

Going through #135 I was thinking what about creating a new folder zipkin-context-cls-hooked and leaving the original zipkin-context-cls as it is.

What do you think?

codefromthecrypt · 2018-04-04T00:00:05Z

@Ghost---Shadow just to keep us from hopping issues too much. what's your idea on doing that? ex the why not the what. I know we've had some debates about how to keep the build running in various fashions and a lot of people have participated in these discussions. Would be nice to summarize work before this, what it solves from previous attempts, what it doesn't solve, how it addresses the build concerns, any compat impact with regards to react native or browser (like angular) apps.

Ghost---Shadow · 2018-04-04T03:04:36Z

@adriancole I am afraid that I am not knowledgeable enough to have opinions on this subject matter.

However, I urgently need this feature for a project so I created a new npm package called zipkin-context-cls-hooked.

If you decide it to merge this as zipkin-context-cls. Then I will deprecate my package and link to this repo.
If you decide to create a new package zipkin-context-cls-hooked then I will unpublish my package and publish the one in this repo, so that npm points to the right thing.

codefromthecrypt · 2018-04-04T04:19:08Z

@adriancole <https://github.com/adriancole> I am afraid that I am not knowledgeable enough to have opinions on this subject matter.

heh you are more knowledgable than me! Main thing I'm hoping for help on is actually getting a product folks can live with. We've had a number of times where something similar is started, but not engaging other discussions and so they end up somewhat mutually distracting. getting feedback > knowledge in other words

…

evan-scott-zocdoc · 2018-04-25T15:44:07Z

Would this fix #88 (comment) in terms of how express async middlewares are handled via CLSContext?

Ghost---Shadow · 2018-04-26T08:34:14Z

Give it a try.

justlaputa · 2018-05-15T10:13:40Z

Hi I'm coming to this issue because I met same problem, may I ask how is this issue now? it will get merged? Thanks!

codefromthecrypt · 2018-06-20T08:03:34Z

@eirslett @DanielMSchmidt considering this: https://medium.com/airbnb-engineering/sunsetting-react-native-1868ba28e30a

do we continue to support (watch out for conflicts with) react native? This topic has locked up a number of issues I'm afraid.

DanielMSchmidt · 2018-06-20T08:20:04Z

@adriancole I would not say React Native is dying just because AirBnB decides to move away from it. That being said, we only have one version of zipkin.js that actually works in a none node environment, the current versions are broken anyway, so why care 🤷‍♂️

I haven't had the time and energy to put up meaningful E2E tests for either the browser or React Native (I think having one of the two would be a good start), sorry about that.

jcchavezs · 2019-03-28T12:47:48Z

Any chance to continue with this @Ghost---Shadow?

Ghost---Shadow · 2019-03-29T09:53:36Z

@jcchavezs I dont even remember what this was :P

I guess not. I have moved on.

EDIT: I made a fork of this repo with this PR. You can find it here.

npm install zipkin-context-cls-hooked

r4j4h · 2019-09-10T19:08:51Z

@Ghost---Shadow where does your fork's code live? The npm page has this project set as its home page and both Google search and Github search are failing me.

zipkin-context-cls-hooked's npm readme even instructs using "zipkin-context-cls", so it feels rather hastily set up and unmaintained. There was conversation #135 (comment) to have it live in https://github.com/openzipkin-contrib but this has not been done.

Just curious about the actual state of this. :)

Ghost---Shadow · 2019-09-11T03:37:13Z

@r4j4h I just forked and fixed it for my personal use, I dont care what others are doing :)

https://github.com/Ghost---Shadow/zipkin-js/tree/bugfix/asyc-await

Anyway, I will resolve the merge conflicts

Ghost---Shadow · 2020-06-05T15:23:36Z

As promised I have deprecated the package

https://www.npmjs.com/package/zipkin-context-cls-hooked

jcchavezs · 2020-06-05T15:36:37Z

Might be too much to ask but would you check if someone is using it and open PRs to that? Do you use it yourself?

…

On Fri, 5 Jun 2020, 17:23 Souradeep Nanda, ***@***.***> wrote: As promised I have deprecated the package https://www.npmjs.com/package/zipkin-context-cls-hooked — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#201 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXOYAQ4ECPLDXKDM6SFXUDRVEEY3ANCNFSM4EYJIHFQ> .

Ghost---Shadow · 2020-06-05T15:49:32Z

@jcchavezs I dont use it anymore. There are 200+ weekly downloads. People are using that.

There are no PRs to my repository.

They should be notified the next time they do an npm install

codefromthecrypt reviewed Apr 3, 2018

View reviewed changes

This was referenced Apr 3, 2018

Any plan for supporting async/await? #175

Closed

Feature request: async_hooks based context #124

Closed

Using cls-hooked

5ec3ceb

jcchavezs mentioned this pull request Jan 31, 2020

feat: adds support for cls-hooked. #476

Merged

jcchavezs closed this Mar 29, 2020

Ghost---Shadow deleted the bugfix/asyc-await branch June 5, 2020 15:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLSContext now supports continuations with async-await #201

CLSContext now supports continuations with async-await #201

Ghost---Shadow commented Mar 31, 2018 •

edited

Loading

codefromthecrypt left a comment

codefromthecrypt Apr 3, 2018

Ghost---Shadow Apr 3, 2018

codefromthecrypt Apr 3, 2018

Ghost---Shadow Apr 3, 2018

codefromthecrypt Apr 3, 2018

Ghost---Shadow commented Apr 3, 2018

codefromthecrypt commented Apr 4, 2018

Ghost---Shadow commented Apr 4, 2018

codefromthecrypt commented Apr 4, 2018 via email

evan-scott-zocdoc commented Apr 25, 2018

Ghost---Shadow commented Apr 26, 2018

justlaputa commented May 15, 2018 •

edited

Loading

codefromthecrypt commented Jun 20, 2018

DanielMSchmidt commented Jun 20, 2018

jcchavezs commented Mar 28, 2019

Ghost---Shadow commented Mar 29, 2019 •

edited

Loading

r4j4h commented Sep 10, 2019

Ghost---Shadow commented Sep 11, 2019 •

edited

Loading

Ghost---Shadow commented Jun 5, 2020

jcchavezs commented Jun 5, 2020 via email

Ghost---Shadow commented Jun 5, 2020 •

edited

Loading

CLSContext now supports continuations with async-await #201

CLSContext now supports continuations with async-await #201

Conversation

Ghost---Shadow commented Mar 31, 2018 • edited Loading

codefromthecrypt left a comment

Choose a reason for hiding this comment

codefromthecrypt Apr 3, 2018

Choose a reason for hiding this comment

Ghost---Shadow Apr 3, 2018

Choose a reason for hiding this comment

codefromthecrypt Apr 3, 2018

Choose a reason for hiding this comment

Ghost---Shadow Apr 3, 2018

Choose a reason for hiding this comment

codefromthecrypt Apr 3, 2018

Choose a reason for hiding this comment

Ghost---Shadow commented Apr 3, 2018

codefromthecrypt commented Apr 4, 2018

Ghost---Shadow commented Apr 4, 2018

codefromthecrypt commented Apr 4, 2018 via email

evan-scott-zocdoc commented Apr 25, 2018

Ghost---Shadow commented Apr 26, 2018

justlaputa commented May 15, 2018 • edited Loading

codefromthecrypt commented Jun 20, 2018

DanielMSchmidt commented Jun 20, 2018

jcchavezs commented Mar 28, 2019

Ghost---Shadow commented Mar 29, 2019 • edited Loading

r4j4h commented Sep 10, 2019

Ghost---Shadow commented Sep 11, 2019 • edited Loading

Ghost---Shadow commented Jun 5, 2020

jcchavezs commented Jun 5, 2020 via email

Ghost---Shadow commented Jun 5, 2020 • edited Loading

Ghost---Shadow commented Mar 31, 2018 •

edited

Loading

justlaputa commented May 15, 2018 •

edited

Loading

Ghost---Shadow commented Mar 29, 2019 •

edited

Loading

Ghost---Shadow commented Sep 11, 2019 •

edited

Loading

Ghost---Shadow commented Jun 5, 2020 •

edited

Loading