WIP: module,errors: port module to internal/errors

[rauno56]

Porting module.js to use internal/errors.js according to tracking issue #11273 and sample PR #11294.

Porting module can be tricky because it's errors are commonly parsed in user-land.

This code sets some properties on caught errors from JSON parsing(example: https://github.com/nodejs/node/blob/master/lib/module.js#L94-L95) should those be left unchanged(as those are really not node libs' errors) or rethrown as new internal error(as we want every error from node to become standardized)?

Additionally to me it makes sense to also leave in setting error.code for backwards compatibility on MODULE_NOT_FOUND error(https://github.com/nodejs/node/blob/master/lib/module.js#L471). Case against that would again be that if we're going to break backwards compatibility, let's just clean it up really nice as we're doing that.

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

Affected core subsystem(s)

errors, module

[ChALkeR]

This probably contradicts with modules being documented as Locked.
Refs: #6528, #11200.

[rauno56]

Got an answer to MODULE_NOT_FOUND concern.
Adding a error code behind a Symbol key doesn't contradict with the Locked status as far as I understand?

[rauno56]

Timers are also Locked, but the fact is not mentioned in the PR touching that.
Refs: #11384

[ChALkeR]

@rauno56 It was mentioned in #11580 that was linked to #11384, but I added an explicit mention there, thanks.

Btw, Locked states:

Stability: 3 - Locked
Only bug fixes, security fixes, and performance improvements will be accepted.
Please do not suggest API changes in this area; they will be refused.

This is not a bugfix, not a security fix, and not a performance improvement, so this technically contradicts with the Locked state to my understanding. There is an ongoing process of removing the Locked state whatsoever, and this PR should ideally be delayed until that is finished.

/cc @nodejs/ctc

[ChALkeR]

@rauno56 module was unlocked in #11661, so you can ignore my above comment now =).

[rauno56]

Perfect! Thank you. I'll finish it up this week.

[refack]

I'll follow up

[BridgeAR]

@refack are you still looking into this / did you open a follow up?

[BridgeAR]

Closing this due to a long inactivity. @rauno56 I am very sorry that your PR could not land as is and your work is much appreciated nevertheless! Please also feel free to leave a comment if you would like to follow up on this by rebasing or by opening a new PR.

[rauno56]

Totally understandable. Sorry for dropping the ball, got busy and discouraged by the delay and confusion in the beginning.

[BridgeAR]

@rauno56 I am very sorry to hear that but I understand your frustration that it took long. For a while there were to many open PRs that it became significantly difficult to keep up with all the PRs. This has improved a lot now and I am certain it reviews would be much faster now.

I am reopening this in the hope you would still want to pursue this further. If not, please let us know as well.

[BridgeAR]

Well, I actually just checked and it seems like it was already ported in the meanwhile.

I am very sorry about this @rauno56