Potential footgun with 'cause' property / not accepting Error

[fowl2]

Consider the following:

function MakeBreakfast() {
    try {
        MakeCoffee();
    } catch (err) {
        throw new Error("Error making breakfast", err);
    }
}

function MakeCoffee() {
    try {
        BoilWater();
    } catch (err) {
        throw new Error("Error making coffee", { cause: err });
    }
}

function BoilWater() {
    throw "Error boiling water";
}

Am I correct in saying that that in code would produce an Error like this

{ 
    message: 'Error making breakfast',
    cause: 'Error boiling water'
}

instead of the intended

{
    message: 'Error making breakfast',
    cause: {
     message: 'Error making coffee',
     cause: 'Error boiling water'
    }
}

?

Due to both the options parameter AND Error having a cause property.

For those familiar with similar features in other languages (eg. C#) this will be an extremely common mistake, and it won't always be immediately obvious as it's just one intermediate "cause" being missing. It'd also be difficult for TypeScript to pick up on a problem like this without explicit.

[fowl2]

Perhaps if the passed in object is an Error, it could be used verbatim instead of its cause property?

[ljharb]

That seems like it'd be very confusing.

I don't think it will be a common mistake; the TypeScript type for Error's second argument is object, and Error isn't that type.

[fowl2]

I can definitely see that magic could be an even worse cure than the disease.

As to the commonness of the mistake - I can find multiple examples in this repo already.

---

On Typescript: Error is assignable to object, and to the presumptive ErrorOptions type that would be created for this proposal.

However, I actually mocked up the likely type definitions, and since Typescript 4.4, the the type of the error in a catch clause is actually unknown, which would not be assignable to ErrorOptions. So crisis averted :)

In my defence this was only changed about a month ago!

So I'm not sure what my conclusion is. Possibly a lint and/or a strong recommendation to ensure that useUnknownInCatchVariables is enabled is enough.

[bakkot]

As to the commonness of the mistake - I can find multiple examples in this repo already.

Those examples (except for the last) predate the change to an options bag, and the last is continuing a discussion which began before that change.

[legendecas]

As the question has already been answered, I'm closing this. Please feel free to re-open if it is not the case.

[fowl2]

I'm not sure it's been answered. I'm fairly convinced this will be a common error - perhaps this could be converted to a discussion for potential mitigations?

[legendecas]

@fowl2 As stated by bakkot above, the examples you pointed out in the repo are outdated and the option bag was introduced afterward. I still don't get your point about how the option bag in the error constructor could be different from all other APIs that using the option bag pattern. The pattern is used widely and I don't believe people who learn a new API in a commonly used pattern would have problems understanding that.