Error.stack is settable on Node and browser but throws error on XS

[cmidgley]

Build environment: Windows Moddable SDK version: 5.0.0 Target device: Win, ESP32

Description

The Error.stack property is not settable on XS but is on Node and browser (Chrome).

This behavior is commonly seen in networking modules (such as on NPM) where they clear the Error.stack properties before sending a serialized object over the wire.

Steps to Reproduce

new Error().stack = "";

Workaround

I work around this today by subclassing Error to extend it with a settable stack and override the Error constructor with my subclass constructor. I don't need a fix/solution. Not sure if you want to know about these types of issues or not... so I erred on the side of over-sharing.

[phoddie]

Thanks for the report. As much as practical, we want to provide behaviors that match the other major engines. Details like this matter.

This XS implementation is based on the Error Stacks proposal. We implemented the getter but clearly overlooked the setter. We'll do that. It looks like that should give the behavior you describe in Node.

[cmidgley]

Another one for you - name is not a setter in XS and causes:

# Exception: set name: not writable (in ResultError)!

class MyError extends Error {
    constructor(message) {
        super(message);
        this.name = "MyError";
    }
}

const err = new MyError("My error message");

Works in Node and browser. Workaround is to define name as a property on the custom class.

Let me know if cases like this you would prefer a new issue especially given you already marked this as confirmed. I'm happy to submit it as a new issue.

[phoddie]

That one is actually not a bug. It is because you are running under Hardened JavaScript, which freezes (hardens) all primordials. If you run this under xst (the XS test tool), it works just fine.

The workaround is to use Object.defineProperty() to set this.name. That isn't going to win prizes for concision, but is how ECMA-262 expects it to work. (Our friends at Agoric call this behavior "the override mistake" and are proposing to modify the behavior under Hardened JavaScript. Plenty written about that elsewhere.)

[cmidgley]

If you mean SES, that's not in use. The code above is run as is from mcconfig -m -d and I get the exception. If you mean something else, I'd appreciate a bit of help.

And, turns out I was wrong about being able to override Error to workaround these issues. Kind of obvious in hindsight ... as Error.prototype is frozen so I can't override the constructor. Before I was testing using my own Error implementation, but I can't do that with third-party modules.

I did for the fun of it try Object.defineProperty(Error.prototype, ...) but that doesn't work on the frozen Error (it works on non-frozen classes). So now I'm in search of a workable solution. Any ideas?

Worth noting, 99% of my modules are preloaded (the only exception right now is @nats-io as it isn't structured to handle that). I did turn off preload, but have the same issues.

[phoddie]

If you mean SES, that's not in use. The code above is run as is from mcconfig -m -d and I get the exception

Hardened JavaScript (formerly SES) executes under lockdown which creates a "frozen realm." The Moddable SDK always runs in that mode -- primordials in immutable ROM.

I did for the fun of it try Object.defineProperty(Error.prototype, ...) but

You can''t modify a frozen object, of course. What I suggested replacing the assignment to this.name with Object.defineProperty(). This does not throw and does set the property:

class MyError extends Error {
    constructor(message) {
        super(message);
        Object.defineProperty(this, "name", {value: "MyErrpr"});
    }
}

const err = new MyError("My error message");

[cmidgley]

I see, thanks for the explanation. I didn't connect Hardened Javascript as the term to use for frozen/immutable implementations.

While that makes sense, I'm confused why stack can get a setter (based on your earlier comment about Error().stack = ''), but name can not. Is it not possible to make name a setter?

Given this, does this mean that third party NPM modules that define a custom Error class with the recommended this.name = 'MyClassName' won't work with XS without forking to use Object.defineProperty?

[phoddie]

While that makes sense, I'm confused why stack can get a setter (based on your earlier comment about Error().stack = ''), but name can not. Is it not possible to make name a setter?

The Error Stack proposal specifies using a getter/setter for the stack property. ECMA-262 specifies Error.prototype.name as an ordinary property. Your project can certainly change that during preload, but the engine should not as it would be non-conforming because changing between an ordinary property and a getter/setter is an observable change.

Given this, does this mean that third party NPM modules that define a custom Error class with the recommended this.name = 'MyClassName' won't work with XS without forking to use Object.defineProperty?

They will not work under Hardened JavaScript. They work with XS in runtimes, like xst, that do not perform lockdown.

There is no need to fork: Object.defineProperty()works in all environments.

As noted above, feel free to review the background on the "override mistake." Exactly this kind of thing has been discussed extensively.

[cmidgley]

Thank you for being so patient with me. I understand Object.defineProperty, but I'm struggling with how to avoid forking/merging when the this.name assignment is in their code. Take this example in @nats-io/nats.js that I use - how can I address this without forking?

[phoddie]

Regarding forking, my point is that a single implementation can support all modes by using Object.defineProperty. If the maintainer of the package chooses not to make that change, patch Error.prototype prior to lockdown (e.g. during preload) with a getter/setter pair for name:

Object.defineProperty(Error.prototype, "name", {
    get: function() {return "Error";},
    set: function(value) {
        Object.defineProperty(this, "name", {value, writable: true, configurable: true});
    }
});

[cmidgley]

Earlier we discussed that we couldn't define a property on Error.prototype, but I tried anyway and took examples/helloworld and pasted this code (using the same manifest):

try {
    Object.defineProperty(Error.prototype, 'name', {
        get: function () {
            return 'Error';
        },
        set: function (value) {
            Object.defineProperty(this, 'name', {
                value,
                writable: true,
                configurable: true,
            });
        },
    });
} catch (e) {
    debugger;
}

and it hits the debugger where e.message is:

invalid descriptor (in defineProperty)

No preload in use, just main.js. Am I missing some detail, perhaps in the manifest? I did run this through Node and it seems to work (though not needed there).

[phoddie]

No preload in use

That's the problem. The note above specifically says that the patch to Error.prototype must occur "during preload".

Here's a main.js...

Object.defineProperty(Error.prototype, "name", {
    get: function() {return "Error";},
    set: function(value) {
        Object.defineProperty(this, "name", {value, writable: true, configurable: true});
    }
});

export default function() {
    class MyError extends Error {
        constructor(message) {
            super(message);
            this.name = "MyError";
        }
    }

    const err = new MyError("My error message");
    trace(err, "\n");
}

..and manifest.json that works:

{
    "include": [ 
        "$(MODDABLE)/examples/manifest_base.json"
    ],
    "modules": {
        "*": "./main"
    },
    "preload": "main"
}

[cmidgley]

OMG - I totally have missed a critical aspect of preload... I've always removed preload to work around frozen/writable issues, but if the objects I'm manipulating belong to XS I have to run my code in preload so I can write to it before they get locked down (doh!)

I'm sure you were getting rather frustrated with me ... but thank you so much for getting me to the other side! I've proposed a change to preload.md (PR #1417) that hopefully captures this.

[phoddie]

Yes, exactly. Glad that's clear now.

The documentation does describe that under Automatic Freezing of Built-ins. Still, wrestling with a behavior can be a better way to understand than reading. ;)

Thanks taking the time to add a detailed example to the docs to highlight the behavior and how to use it. That will surely help others.