fix: missing stack starting from node 21+ #398
Conversation
| @@ -86,6 +87,16 @@ module.exports = internals.clone = function (obj, options = {}, _seen = null) { | |||
| continue; | |||
| } | |||
|
|
|||
| // Can only be covered in node 21+ | |||
There was a problem hiding this comment.
structuredClone is available from v18.x. v21 is the one that breaks the stack property copying old behaviour.
There was a problem hiding this comment.
Does it matter if we use structuredClone early though? I guess it can't hurt, right?
There was a problem hiding this comment.
Functionally I would not expect it to, though it might change the performance characteristics. It could be significantly slower (eg. from changing the prototype). Hopefully something like hapijs/boom#304 will be part of the new releases to avoid the copying altogether.
There was a problem hiding this comment.
This appears to break boom when using jest, because for jest structuredClone(new Error('test')) instanceof Error is false 🤦🏻♂️ Not a problem for everyone, but I expect some bug reports...
There was a problem hiding this comment.
This appears to break boom when using jest, because for jest
structuredClone(new Error('test')) instanceof Erroris false 🤦🏻♂️ Not a problem for everyone, but I expect some bug reports...
That does not seem true in general. Especially because we override the prototype if it doesn't match the input. How are you using it?
There was a problem hiding this comment.
Oh, nvm. The issue is that the VM environment that it runs can cause instanceof Error to just fail.
The primary workaround seems to be not using the VM through jest-light-runner, or alternatively this might work.
There was a problem hiding this comment.
Indeed, I had to switch to the latter on my project, it's not difficult but not something I expected to do either. Anyway, I think we can expect some complaints.
There was a problem hiding this comment.
FYI, this solution should still work within the VM, as we fix the prototype (I verified that it works under jest), so I'm still not certain why you observe a problem.
There was a problem hiding this comment.
I don't know if it's specific to my computer, but creating an entirely new project with only jest and boom with a super simple test already fails, yet I can't reproduce that in stackblitz for example 🤷🏻♂️
EDIT: erratum, it obviously happened in our CI as well, so it's not just me. Maybe linux-specific?
There was a problem hiding this comment.
I figured it out! We assume that the cloned class is the same as the original class, which for plain Errors under this kind of VM is not true. I have made a fix for master in #399.
Backporting #390 to speed up that fix. I just did feature detection based on
structuredClone's presence.