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

Assertion error starting from v14.0.0 #33334

Closed
ghost opened this issue May 10, 2020 · 2 comments
Closed

Assertion error starting from v14.0.0 #33334

ghost opened this issue May 10, 2020 · 2 comments

Comments

@ghost
Copy link

ghost commented May 10, 2020

Our code:

const assert = require('assert');

module.exports = new Proxy({}, {
  getPrototypeOf(){
    assert.fail();
  }
});

We want to export a proxified object and explicitly ensure that user code will not trigger certain traps. Works fine in v12.x. Throws assertion error in v14.x, but we did not call getPrototypeOf.

We bisected to d7452b7. Throws error even with --no-warnings.

Please Node.js team, can you fix this? Thanks.

@rickyes
Copy link
Contributor

rickyes commented May 10, 2020

} else if (module.exports &&
ObjectGetPrototypeOf(module.exports) ===
CircularRequirePrototypeWarningProxy) {
ObjectSetPrototypeOf(module.exports, PublicObjectPrototype);
}
}

It looks like getPrototypeOf is called after successful module loading.

@rickyes
Copy link
Contributor

rickyes commented May 10, 2020

Maybe we could use --no-warnings without checking CircularRequirePrototypeWarningProxy.

/cc @addaleax

BridgeAR added a commit to BridgeAR/node that referenced this issue May 11, 2020
In case the exported module is a proxy that has the `getPrototypeOf`
or `setPrototypeOf` trap, skip the circular dependencies check.
It would otherwise be triggered by the check itself.

Fixes: nodejs#33334

Signed-off-by: Ruben Bridgewater <[email protected]>
codebytere pushed a commit that referenced this issue May 16, 2020
In case the exported module is a proxy that has the `getPrototypeOf`
or `setPrototypeOf` trap, skip the circular dependencies check.
It would otherwise be triggered by the check itself.

Fixes: #33334

Signed-off-by: Ruben Bridgewater <[email protected]>

PR-URL: #33338
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
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 a pull request may close this issue.

1 participant