Error.stackTraceLimit becomes undefined when running from snapshot

[acutmore]

Version

v22.8.0

Platform

Darwin KGVY6KLQ1J 23.6.0 Darwin Kernel Version 23.6.0: Mon Jul 29 21:13:04 PDT 2024; root:xnu-10063.141.2~1/RELEASE_ARM64_T6020 arm64

Subsystem

No response

What steps will reproduce the bug?

Description

Error.stackTraceLimit is undefined while the snapshot is being run, meaning code which "resets" stackTraceLimit after temporarily modifying it is consequently setting it to undefined which is persisted when resuming from the snapshot.

I hit this when importing typescript while building the snapshot.

Setup

Create this file:

// app.js
function warmup() {
    // simplified version of https://github.com/microsoft/TypeScript/blob/fa0080f4802fd78fb0f01cd0160f299794d7843d/src/compiler/sys.ts#L1897-L1919
    const previousLimit = Error.stackTraceLimit;
    try {
        Error.stackTraceLimit = 1;
        // ...do stuff
    } finally {
        Error.stackTraceLimit = previousLimit;
        console.log("warmup: Error.stackTraceLimit is ", Error.stackTraceLimit);
    }
}

function task() {
    const err = new Error();
    console.log("task: typeof err.stack is ", typeof err.stack);
}


const v8 = require("v8");
if (v8.startupSnapshot.isBuildingSnapshot()) {
    warmup();
    v8.startupSnapshot.setDeserializeMainFunction(task);
} else {
    warmup();
    task();
}

No snapshots:

> node app.js
warmup: Error.stackTraceLimit is 10
task: typeof err.stack is string

With snapshots:

> node --snapshot-blob=snap.blob --build-snapshot app.js
warmup: Error.stackTraceLimit is  undefined
Deleting Error.stackTraceLimit from the snapshot. It will be re-installed after deserialization
> node -snapshot-blob=snap.blob app.js
task: typeof err.stack is  undefined

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

100%

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

I was expecting Error.stackTraceLimit to have the same starting value regardless of if --build-snapshot is passed.

As a result I was also expecting error.stack within task to be defined when running from the snapshot, because it is defined when running without the snapshot.

What do you see instead?

Error.stackTraceLimit is undefined when running with --build-snapshot.

As a result typeof error.stack is string when running the code normally and undefined when using a snapshot.

Additional information

Startup snapshots parent issue: #44014

error.stack being undefined while building the snapshot - I presume this is there as a security precaution to avoid leaking implementation details of the file system into the snapshot? Maybe this could be documented? (apologies if this is already the case)

I wonder if the logic for restoring Error.stackTraceLimit should only happen if the snapshot builder explicitly sets Error.stackTraceLimit to a number and not to undefined - because undefined is the starting state it hasn't actually been changed by the snapshot builder.

The workaround I have in place is to explicitly set Error.stackTraceLimit = 10; before exiting the snapshot builder. This works, however it would be nice to not hard-code the size and instead allow it to retain the default. EDIT: delete Error.stackTraceLimit; at the end of the snapshot building seems to be a 'better' workaround.

[joyeecheung]

We are currently manually deleting Error.stackTraceLimit before snapshot serialization in here

node/lib/internal/main/mksnapshot.js

Lines 149 to 159 in f29d2b7

	addSerializeCallback(() => {
	stackTraceLimitDesc = ObjectGetOwnPropertyDescriptor(Error, 'stackTraceLimit');
	if (stackTraceLimitDesc !== undefined) {
	// We want to use null-prototype objects to not rely on globally mutable
	// %Object.prototype%.
	ObjectSetPrototypeOf(stackTraceLimitDesc, null);
	process._rawDebug('Deleting Error.stackTraceLimit from the snapshot. ' +
	'It will be re-installed after deserialization');
	delete Error.stackTraceLimit;
	}

Some background on this behavior: when V8 deserialize the snapshot, it will try to re-install Error.stackTraceLimit (which can vary depending on --stack-trace-limit), so we are left with two options:

1. During deserialization, don't try to re-install the limit if it's already in the snapshot, and just keep whatever that's in the snapshot, which I tried to upstream in https://chromium-review.googlesource.com/c/v8/v8/+/3319481
2. During serialization, remove whatever limit that's configured in the snapshot builder script, so that V8 can re-install it during deserialization - this was suggested by the V8 team in the aforementioned CL, and was what I implemented in bootstrap: fixup Error.stackTraceLimit for user-land snapshot #44203

It does seems strange though why the limit isn't re-initialized to 10 (the default of --stack-trace-limit) after snapshot deserialization, I'll debug a bit to find out.

[joyeecheung]

So V8 doesn't actually install Error.stackTraceLimit when the context is created for building a snapshot (in addition V8 doesn't install several APIs, most importantly WebAssembly, when it's in a context created for snapshot building). The serialization callback we have just tries to restore the user-mutated context back to the shape that the V8 context deserializer expects. I think we have a few options:

1. Try to convince V8 to add a mode to initialize Error.stackTraceLimit and/or WebAssembly etc. for contexts created for snapshot building, so that the builder scripts can use them
2. Initialize Error.stackTraceLimit from --stack-trace-limit ourselves during snapshot building (I don't think we can do much about WebAssembly, unless we want to re-implement it using the V8 API, which doesn't seem very feasible, and also I suspect V8 just doesn't support snapshotting compiled WebAssembly anyway)
3. Just documents that Error.stackTraceLimit and WebAssembly are not available in snapshot builder scripts. Users can fix Error.stackTraceLimit up themselves if needed before using the limit, but it will be re-initialized by V8 using --stack-trace-limit value upon deserialization, unless users have another deserialization callback to override it after the V8 initialization.

3 would be the easiest, I am skeptical whether 1 would actually be accepted by the upstream.

[acutmore]

2 (specifically the stackTraceLimit, not webassembly) sounds good to me. If there aren't security concerns about having stack traces enabled by default during the snapshot building. This feels user friendly for getting started with snapshots.

3 also sounds good if 2 isn't viable.