assert.ok() throwing AssertionError instead of provided Error object

Version

v20.9.0

Platform

Linux executive 6.5.0-10-generic #10-Ubuntu SMP PREEMPT_DYNAMIC Fri Oct 13 13:49:38 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

assert

What steps will reproduce the bug?

assert.ok() docs says:

If the message parameter is an instance of an Error then it will be thrown instead of the AssertionError.

I've called to it with an Error instance as second argument, and instead of being thrown that error, it's being thrown an AssertionError with it's message field set to the message field of the provided error.

Not sure what's the correct solution here, based on docs and common sense, the provided error should be thrown, but based on homogeneity, an AssertionError should be thrown (current behaviour) so it always throw an AssertionError no matter what it's provided as second argument...

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

Always.

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

Not sure if it's a bug, or a documentation issue.

What do you see instead?

AssertionError is thrown, with provided Error message.

Additional information

No response

node/lib/assert.js

Lines 396 to 398 in 25fdb48

    
           } else if (message instanceof Error) { 
        
             throw message; 
        
           }

It clearly re-thrown if the message is instanceof Error, but instanceof checking may not be true depends on testing framework.

A minimal repo shown it is working correctly.

import { ok } from 'assert/strict'

ok(false, Error('here is error'))

This working as expected and will throw to avoid reading a Node.js module, as doing so could potentially result in misleading errors being thrown from module.

instanceof checking may not be true depends on testing framework

My fault... I'm using Jest, and forget it has problems with instanceof and builtin classes 🤦🏻. Could make it sense to use ducktyping and check for Error objects fields? Based on spec, message, name and stack can made us pretty sure they are Error objects themselves...

We could use the same algorithm as util.isError:

node/lib/util.js

Lines 167 to 169 in 1858341

    
           function isError(e) { 
        
             return ObjectPrototypeToString(e) === '[object Error]' || e instanceof Error; 
        
           }

We could use the same algorithm as util.isError

It's deprecated, and also it would not work for Error child classes.

The function is deprecated, but not what it does. It works fine with child classes.

It works fine with child classes.

I think it wouldn't, because on child classes, the prototype constructor name would be different of Error string, so the comparison would fail and we would get to the e instanceof Error, leading to the same Jest error we have here in this issue.

If you don't believe me, try it

We can also add a check that looks util.types.isNativeError(). That is how assert itself checks for errors next to instanceof.

I like the idea.

Hello, I would like to try and work on this issue.

What do you need?

Seeing the discussion, I will try to implement both the methods mentioned to see if it actually throws the required error. Will update accordingly?

Hi, can I work on this issue?

Hi, can I work on this issue?

Sure, go for it :-)

is this issue still open ?

Yes, it's still a problem.

I noticed that this issue is stalled even though there is an almost accepted PR that solves the problem.
I addressed the comments and fixed the problem that was blocking the CI (we'll find out on the next CI run, hehe 🐍 ).

PR: #53980

P.S.: I added the author of the PR and the commenter as co-authors in the commit.

assert: use isError instead of instanceof in innerOk

@pmarchini

Great, good work @pmarchini :-)

assert: use isError instead of instanceof in innerOk

Navigation Menu

Repository navigation

`assert.ok()` throwing `AssertionError` instead of provided `Error` object #50780

Version

Platform

Subsystem

What steps will reproduce the bug?

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

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

What do you see instead?

Additional information

9 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	function isError(e) {
	return ObjectPrototypeToString(e) === '[object Error]' \|\| e instanceof Error;
	}

assert.ok() throwing AssertionError instead of provided Error object #50780

Description

Version

Platform

Subsystem

What steps will reproduce the bug?

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

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

What do you see instead?

Additional information

Activity

climba03003 commented on Nov 18, 2023

MrJithil commented on Nov 21, 2023

piranna commented on Nov 22, 2023

targos commented on Nov 22, 2023

piranna commented on Nov 22, 2023

targos commented on Nov 22, 2023

piranna commented on Nov 22, 2023

targos commented on Nov 22, 2023

BridgeAR commented on Dec 18, 2023

piranna commented on Dec 18, 2023

NiharPhansalkar commented on Dec 20, 2023

piranna commented on Dec 20, 2023

NiharPhansalkar commented on Dec 20, 2023

9 remaining items

ignaciosuarezquilis commented on May 6, 2024

piranna commented on May 7, 2024

Naveen-2021ucp1387 commented on May 15, 2024

piranna commented on May 15, 2024

pmarchini commented on Jul 21, 2024

piranna commented on Jul 28, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

`assert.ok()` throwing `AssertionError` instead of provided `Error` object #50780