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

Function serialization broken in NodeJS 19.2 #11488

Closed
kpitzen opened this issue Nov 29, 2022 · 3 comments · Fixed by #11932
Closed

Function serialization broken in NodeJS 19.2 #11488

kpitzen opened this issue Nov 29, 2022 · 3 comments · Fixed by #11932
Assignees
Labels
area/callback-functions aka function serialization kind/bug Some behavior is incorrect or out of spec language/javascript resolution/fixed This issue was fixed
Milestone

Comments

@kpitzen
Copy link
Contributor

kpitzen commented Nov 29, 2022

Update on issue

If you encounter this error, update to Node 19.4 or newer.


What happened?

When opening a new PR unrelated to the Node SDK, tests related to function serialization fail due to a missing internal property [[Scopes]]

Steps to reproduce

Run Node SDK unit tests in CI

Expected Behavior

If no code changes have been made, no tests should fail.

Actual Behavior

Tests are failing

Output of pulumi about

Not terribly relevant, but:

CLI
Version 3.46.2-dev.0
Go Version go1.19.2
Go Compiler gc

Host
OS ubuntu
Version 20.04
Arch x86_64

Backend
Name pulumi.com
URL https://app.pulumi.com/kpitzen
User kpitzen
Organizations kpitzen, EpicGames, pulumi

Additional context

Here is the original failing job:
https://github.com/pulumi/pulumi/actions/runs/3577856744/jobs/6017440059

And a no-op job with only a README change:
https://github.com/pulumi/pulumi/actions/runs/3578273088/jobs/6018337627

The failing branch of code ends here:

const scopes = internalProperties.find(p => p.name === "[[Scopes]]");

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@kpitzen kpitzen added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Nov 29, 2022
@Frassle Frassle added p0 A bug severe enough to interrupt existing work area/callback-functions aka function serialization language/javascript and removed needs-triage Needs attention from the triage team labels Nov 30, 2022
@Frassle
Copy link
Member

Frassle commented Nov 30, 2022

Well would you look at that, Node.js 19.2.0 dropped yesterday 2022-11-29.

I'm guessing that's changed some internal details, we better make sure we're testing on 19.2 and some older versions for this.

@Frassle
Copy link
Member

Frassle commented Nov 30, 2022

Node 19.2 updated to a newer version of V8, and it looks like V8 has started to remove that "[[Scopes]]" property: v8/v8@735401e

This stuff hasn't been touched for four years, and I'm not sure what we could do to lookup captured variables without going through [[Scopes]].

Given this is hard blocking everything I'm going to state that we do not support NodeJS 19.2 and higher for the time being. I'll update docs and CI quickly now to get pipelines moving, and then we need to work out if we can fix this.

bors bot added a commit that referenced this issue Nov 30, 2022
11490: Limit NodeJS to 19.1 r=Frassle a=Frassle

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

Workaround for #11488

Co-authored-by: Fraser Waters <fraser@pulumi.com>
bors bot added a commit that referenced this issue Nov 30, 2022
11490: Limit NodeJS to 19.1 r=Frassle a=Frassle

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

Workaround for #11488

Co-authored-by: Fraser Waters <fraser@pulumi.com>
bors bot added a commit that referenced this issue Nov 30, 2022
11490: Limit NodeJS to 19.1 r=Frassle a=Frassle

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

Workaround for #11488

Co-authored-by: Fraser Waters <fraser@pulumi.com>
@Frassle Frassle changed the title (sdks/nodejs) Function serialization tests failing in CI Function serialization broken in NodeJS 19.2 Nov 30, 2022
@mikhailshilkov mikhailshilkov added p1 A bug severe enough to be the next item assigned to an engineer and removed p0 A bug severe enough to interrupt existing work labels Dec 1, 2022
@AaronFriel
Copy link
Contributor

I believe we can take the p1 label off of this, as I've reached out to Chromium engineers and they will revert and cherry pick the revert into the next V8 release. See: https://bugs.chromium.org/p/chromium/issues/detail?id=1365858#c13

I will file an issue with Node requesting they ensure this is cherry-picked during the 19.x release cycle.

@mikhailshilkov mikhailshilkov removed the p1 A bug severe enough to be the next item assigned to an engineer label Dec 14, 2022
AaronFriel added a commit to pulumi/pulumi-hugo that referenced this issue Jan 20, 2023
The upstream issue should be resolved, with details in pulumi/pulumi#11932 and pulumi/pulumi#11488.

This should be merged after pulumi/pulumi#11932 has merged in case there are any other issues with Node current.
bors bot added a commit that referenced this issue Jan 20, 2023
11932: ci: Resume testing on Node current with 19.4 release r=justinvp a=AaronFriel

Fixes #11488. 

Upstream PR to Node was merged and included in 19.4: https://github.com/nodejs/node/releases/tag/v19.4.0

Co-authored-by: Aaron Friel <mayreply@aaronfriel.com>
@bors bors bot closed this as completed in cee5191 Jan 20, 2023
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Jan 20, 2023
AaronFriel added a commit to pulumi/pulumi-hugo that referenced this issue Jan 20, 2023
The upstream issue should be resolved, with details in pulumi/pulumi#11932 and pulumi/pulumi#11488.

This should be merged after pulumi/pulumi#11932 has merged in case there are any other issues with Node current.
@mikhailshilkov mikhailshilkov added this to the 0.83 milestone Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/callback-functions aka function serialization kind/bug Some behavior is incorrect or out of spec language/javascript resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants