Hardened JavaScript interferes with Node.js 14 Error construction
@warner reports:
Then I used fs.openReadStream(undefined) (not intentionally, I forgot to provide argv). The error I get is T
ypeError#1: Cannot assign to read only property 'name' of object 'TypeError: The "path" argument must be of type string or an instance of Buffer or URL. Received undefined'the stack trace includes:at addCodeToName (internal/errors.js:329:12) at new NodeError (internal/errors.js:294:7) at internal/fs/utils.js:634:11
The relevant code in Node.js 14.17.4 is:
function addCodeToName(err, name, code) {
// Set the stack
if (excludedStackFn !== undefined) {
ErrorCaptureStackTrace(err, excludedStackFn);
}
// Add the error code to the name to include it in the stack trace.
err.name = `${name} [${code}]`;
// Access the stack to generate the error message including the error code
// from the name.
err.stack; // eslint-disable-line no-unused-expressions
// Reset the name to the actual name.
if (name === 'SystemError') {
ObjectDefineProperty(err, 'name', {
value: name,
enumerable: false,
writable: true,
configurable: true
});
} else {
delete err.name;
}
}
And the culprit is the property override hazard, upon assigning to name of an instance descended from a frozen error constructor.
Although #867 fixes the issue revealed here, it won't actually change this specific occurrence. The problematic Node code might override the name property of any error by assignment. #867 extends the default overrideTaming: 'moderate' setting to enable override-by-assignment of name for all error types. But it was already enabled for Error and TypeError, so this symptom would not have been triggered at the 'moderate' setting. We only became aware of it by debugging at the overrideTaming: 'min' setting. At that setting, this problem will still occur.
@kriskowal Can you please put an estimate on this?
@erights I’m not clear on whether or what the next steps for this issue would be. Is there fix we could stomach that would address this issue at all overrideTaming settings?
Well, we could trivially copy this moderate change so it applies to min too. I don't like it, but Node....
Have we told Node about this? Have we filed an issue? Is it still a problem on Node 16?
@kriskowal This does not have an area label that is covered by our weekly tech / planning meetings. Can you assign the proper label? We cover: agd, agoric-cosmos, amm, core economy, cosmic-swingset, endo, getrun, governance, installation-bundling, metering, run-protocol, staking, swingset, swingset-runner, token economy, wallet, zoe contract,
We no longer support Node.js 14.