Tape: t.throws throws with "Cannot delete property"

[dcousens]

Reference: https://github.com/substack/tape/blob/master/lib/test.js#L448

        delete err.message;
               ^

TypeError: Cannot delete property 'message' of Error
    at Test.throws (../node_modules/tape/lib/test.js:448:16)

This seems like really odd behaviour and doesn't work with custom errors?

[ljharb]

https://github.com/substack/tape/commit/f66724fc98e6bede53afa54ca83521bdc50f8a2f implies that the purpose is ensuring that the property is enumerable (even in ES3).

It should work fine with custom errors, unless their "message" property is nonconfigurable for some reason. What test code is creating this error?

[dcousens]

The error is coming from https://github.com/dcousens/typeforce/blob/master/index.js#L13-L20, as I'm putting that repository under tape at the moment.

Even ensuring the property is enumerable: true doesn't solve my issue?
It is lazily evaluated due to tfErrorString performing JSON.stringify a few times.

[ljharb]

So, a few reactions:

• you should probably use Object.defineProperty inside your getter to change it into a data property once it's cached instead of making it always be a (slow) getter
• since the goal of that delete + reassign of "message" was enumerability, I think a) it should not do that if the property is already enumerable, and b) it should detect if the property is nonconfigurable, and avoid attempting to modify it if that's the case.

[dcousens]

you should probably use Object.defineProperty inside your getter to change it into a data property once it's cached instead of making it always be a (slow) getter

That was my first thought, but node 6 doesn't allow that? (or am I doing something else wrong?)

TypeError: Cannot redefine property: message

since the goal of that delete + reassign of "message" was enumerability, I think a) it should not do that if the property is already enumerable, and b) it should detect if the property is nonconfigurable, and avoid attempting to modify it if that's the case.

Couldn't you just check that via:

assert(Object.keys(err).some(x => x === 'message'))

[ljharb]

Even easier, and ES3-compatible, with Object.prototype.isEnumerable.call. I'll make that change tomorrow, at least.

Because configurable defaults to false, you'll have to change the getter defineProperty to have configurable: true to be able to redefine it later.

[dcousens]

Thanks mate, so for tomorrows change, I'll be able to get away with changing: enumerable: true for my purposes?

[ljharb]

Yep, that's the idea.

[dcousens]

Thanks @ljharb :)