[8.x] assert: revert breaking change

[BridgeAR]

It was not intended to change the assert.doesNotThrow() message
with #23223. This reverts the
breaking behavior to the one before.

Refs: #23223 (comment)

I guess it won't hurt much if we keep this change in 8.x but it was not meant to be
in 8 and therefore I opened this revert.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[nodejs-github-bot]

@BridgeAR build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1795/pipeline

[BridgeAR]

FYI @nodejs/release

[BethGriggs]

I'd be +1 on reverting

[rvagg]

This is in 8.x releases now? For how long? Fixing it is going to be another breaking change for folks who have adapted to the change. Since we've also done a security release I would hope the majority of users are on the latest version so have already adapted.
I'm inclined to leave it. Maybe even go back to the release blog post and put an update note about it (just a line in italics with the of the note).

[BridgeAR]

It was released in 8.13 which is not yet two weeks old. I honestly have no strong opinion either way. So maybe we should just vote?

🎉 revert
👍 keep the breaking change

[sam-github]

The actual change: https://github.com/nodejs/node/pull/24786/files

Summary: it appends more info to the .message property of the error thrown from an assert.

A nice change, but changes to .message are defined as semver-major by policy. Should revert, IMO.

[BridgeAR]

Let's wait 24h more hours but I suggest to have a new patch release very soon in case we decide to revert.

[BridgeAR]

There are 4 collaborators who would like to revert and 1 who would like to keep the current change. Since it's out there for two weeks only, I think it's best to revert this.

@BethGriggs AFAIK you'll prepare the next 8.x.x release: would you be so kind and include this PR in that case as well?

[MylesBorins]

@BridgeAR I think what @rvagg meant was that it went out with the v8.14.0 release, which as a security release likely had more upgrades then a normal release. As such people may have already adapted.

Alternatively, unexpected breaking changes could stop people from adopting a security release.

I genuinely doubt that this is breaking lots of production stuff, although I imagine it could break some tests in an annoying way.

I've kicked off a CI: https://ci.nodejs.org/job/node-test-pull-request/192561

If it is green and there are no objections I think we should consider landing it. It would be nice to have more explicit sign-off before doing so.

[BridgeAR]

CI is green.

[BethGriggs]

Landed in 62fb5db