+1 for semver-patch. IIRC the point of these new error objects is that changes in the error message can be done without semver-major because of the existence of code.

Anyone with more experience then me, feel free to downgrade semver level.

Effectively blocked by #13739, see #13730 (comment)

@tniessen I believe we still try to give 72 hours for review during weekends instead of the 48 hours given during the week.

@mscdex My bad, I scheduled it for Tuesday... You are right, that's 72 hours, not 48.

In addition to the current change I would like to also check if the provided type is actually a primitive or a class (either with a whitelist of primitives or by checking if the first letter is upper case) and change the error message accordingly. If it's a class we could write instance of instead of type of and also print the provided constructor name in case the value was provided and it is of type object. The one in "one of" could also be removed if I'm not mistaken.

function test (opts) {
  if (!isUint8Array(opts.prop))
    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'opts.prop' ['Buffer', 'Uint8Array'], opts.prop)
}

test({ prop: [] })
// TypeError [ERR_INVALID_ARG_TYPE]: The "opts.prop" property must be instance of Buffer or Uint8Array. Received instance of Array.
// instead of
// TypeError [ERR_INVALID_ARG_TYPE]: The "opts.prop" property must be one of type Buffer or Uint8Array. Received type object

test({ prop: 'string' })
// TypeError [ERR_INVALID_ARG_TYPE]: The "opts.prop" property must be instance of Buffer or Uint8Array. Received type string.
// instead of
// TypeError [ERR_INVALID_ARG_TYPE]: The "opts.prop" property must be one of type Buffer or Uint8Array. Received type string

Should I just update the PR or open another one for that?

Neither the current change nor the proposed one should be semver major though as the error codes do not change and the messages are meant to change.

This does not need to be semver-major.

@BridgeAR I would prefer a separate PR to discuss such changes.

Please rebase this on master and remove any modifications to test/parallel/test-process-hrtime.js (basically squash the second commit). CI won't run cleanly as it cannot automaticall merge the first commit.

Learned a new thing about error message semverity.
Ping @tniessen should this be un-blocked?

@tniessen done

Pre land CI: https://ci.nodejs.org/job/node-test-commit/10693/

Landed in 3e17884

@jasnell Can you explain why it's not semver-major?

It does not change any error types or the error code. The changes are limited to the error message and only in certain cases. Per the guidelines for the new internal/errors, changes to the message are not semver-major once the error has been migrated to use codes.

@jasnell Why treat the error type specially (assuming you're referring to use of Error, TypeError, RangeError, etc.) and not the message? I would think users could be explicitly checking both?

Also, I'm kind of surprised we're not giving any grace period for users to switch after the new errors system is rolled out (everywhere).

Also, I'm kind of surprised we're not giving any grace period for users to switch after the new errors system is rolled out (everywhere).

I'm assuming since this changed only internal errors, and the change over to internal errors is semver-major the impact is mitigated.

I'm assuming since this changed only internal errors

@refack I don't understand what you mean by 'internal' errors. This PR changes error messages received by end users of node.

I don't understand what you mean by 'internal' errors

The new errors that must include .code as tracked by #11273. Those errors' messages were already changed in a semver-major way.

@refack Typically the messages aren't changed (initially) but what I said is still relevant:

Also, I'm kind of surprised we're not giving any grace period for users to switch after the new errors system is rolled out (everywhere).

Temporarily "don't land"ing this until semverity is decided.