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

TypeError: Method Map.prototype.set called on incompatible receiver #<AsyncContextFrame> #54503

Closed
bengl opened this issue Aug 22, 2024 · 2 comments · Fixed by #54510
Closed
Labels
async_hooks Issues and PRs related to the async hooks subsystem. repro-exists Issues with reproductions.

Comments

@bengl
Copy link
Member

bengl commented Aug 22, 2024

Version

v22.7.0

Platform

Darwin poutine-mac 23.6.0 Darwin Kernel Version 23.6.0: Fri Jul  5 17:56:41 PDT 2024; root:xnu-10063.141.1~2/RELEASE_ARM64_T6000 arm64 arm Darwin

Subsystem

async_hooks

What steps will reproduce the bug?

node --experimental-async-context-frame -e "new async_hooks.AsyncLocalStorage().run({}, () => {})"

How often does it reproduce? Is there a required condition?

100% always happens.

What is the expected behavior? Why is that the expected behavior?

The above command should just exit cleanly with no output.

What do you see instead?

node:internal/async_context_frame:56
    this.set(store, data);
         ^

TypeError: Method Map.prototype.set called on incompatible receiver #<AsyncContextFrame>
    at AsyncContextFrame.set (<anonymous>)
    at new AsyncContextFrame (node:internal/async_context_frame:56:10)
    at AsyncLocalStorage.enterWith (node:internal/async_local_storage/async_context_frame:24:19)
    at AsyncLocalStorage.run (node:internal/async_local_storage/async_context_frame:30:10)
    at [eval]:1:37
    at runScriptInThisContext (node:internal/vm:209:10)
    at node:internal/process/execution:118:14
    at [eval]-wrapper:6:24
    at runScript (node:internal/process/execution:101:62)
    at evalScript (node:internal/process/execution:136:3)

Node.js v22.7.0

Additional information

While the test suite includes the running of async-local-storage tests with the flag enabled, it doesn't actually check whether those files actually passed or failed. It just runs them. That's why this wasn't caught.

I think I have a fix ready. I'm just waiting on a compile to verify.

@RedYetiDev RedYetiDev added the async_hooks Issues and PRs related to the async hooks subsystem. label Aug 22, 2024
bengl added a commit to bengl/node that referenced this issue Aug 22, 2024
This gives a class prototype for AsyncContextFrame that contains the
required methods, so that when we swap the prototype,
ActiveAsyncContextFrame methods are used instead. Previously, the
methods were defined in AsyncContextFrame, so swapping the prototype
didn't swap those static methods.

Also, make the ActiveAsyncContextFrame extend from Map.

Fixes: nodejs#54503
@andreialecu
Copy link

I was excited about trying --experimental-async-context-frame to check if it helps with open telemetry tracing overhead, and I just ran into this issue.

CleanShot 2024-08-23 at 15 39 05@2x

@RedYetiDev RedYetiDev added the repro-exists Issues with reproductions. label Aug 23, 2024
@RedYetiDev
Copy link
Member

$ node --experimental-async-context-frame -e "new async_hooks.AsyncLocalStorage().run({}, () => {})" 
node:internal/async_context_frame:56
    this.set(store, data);
         ^

TypeError: Method Map.prototype.set called on incompatible receiver #<AsyncContextFrame>
    at AsyncContextFrame.set (<anonymous>)
    at new AsyncContextFrame (node:internal/async_context_frame:56:10)
    at AsyncLocalStorage.enterWith (node:internal/async_local_storage/async_context_frame:24:19)
    at AsyncLocalStorage.run (node:internal/async_local_storage/async_context_frame:30:10)
    at [eval]:1:37
    at runScriptInThisContext (node:internal/vm:209:10)
    at node:internal/process/execution:118:14
    at [eval]-wrapper:6:24
    at runScript (node:internal/process/execution:101:62)
    at evalScript (node:internal/process/execution:136:3)

Node.js v22.7.0

sendoru pushed a commit to sendoru/node that referenced this issue Sep 1, 2024
This gives a class prototype for AsyncContextFrame that contains the
required methods, so that when we swap the prototype,
ActiveAsyncContextFrame methods are used instead. Previously, the
methods were defined in AsyncContextFrame, so swapping the prototype
didn't swap those static methods.

Also, make the ActiveAsyncContextFrame extend from Map.

Fixes: nodejs#54503
PR-URL: nodejs#54510
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
aduh95 pushed a commit that referenced this issue Sep 12, 2024
This gives a class prototype for AsyncContextFrame that contains the
required methods, so that when we swap the prototype,
ActiveAsyncContextFrame methods are used instead. Previously, the
methods were defined in AsyncContextFrame, so swapping the prototype
didn't swap those static methods.

Also, make the ActiveAsyncContextFrame extend from Map.

Fixes: #54503
PR-URL: #54510
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. repro-exists Issues with reproductions.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants