update: add blackbox by default

[RafaelGSS]

Reference: nodejs/node#11893

This PR is only for node 15 >. (When internal modules are prefixed by node:).

Observations

Settings:

node: v15.6.0
DISTRIB_ID=LinuxMint
DISTRIB_RELEASE=20
DISTRIB_CODENAME=ulyana
DISTRIB_DESCRIPTION="Linux Mint 20 Ulyana"
NAME="Linux Mint"
VERSION="20 (Ulyana)"

Application example:

'use strict';constfs=require('fs');fs.readFile(__filename,function(){});

Without this change:

node inspect example.js
debug> n
debug> s
breakin node:fs:322
320
321 functionreadFile(path, options, callback) {
>322 callback = maybeCallback(callback || options);
323 options = getOptions(options, { flag: 'r' });
324 if (!ReadFileContext)

On change:

node inspect example.js
debug> n
debug> s
breakin node:internal/validators:218
216
217 const validateCallback = hideStackFrames((callback) => { >218 if (typeof callback !== 'function')219 throw new ERR_INVALID_CALLBACK(callback);220 });

---

Not sure if it's expected a break inside node:internal/* when the pattern ^node: is passed to the blackbox script patterns, may it can be a bug inside pattern matching?

cc: @nodejs/diagnostics

[RafaelGSS]

Not sure if it's expected a break inside node:internal/* when the pattern ^node: is passed to the blackbox script patterns, may it can be a bug inside pattern matching?

As discussed in WG Meeting, we don't know exactly whether that behavior is expected. Does someone have any thoughts about it? @jkrems?

[RafaelGSS]

Removing diag-agenda label since there are some investigations to do regarding prefix all core modules.

[Trott]

I'm trying to figure out if this should be applied to the current 16.x implementation. I think not, but I also don't understand why it would apply to 15.x, so I'm missing some context. Can someone help me?

I'd assume that stepping into the fs.readfile() call would take one to the implementation of fs.readfile() in fs.js as it does in the current implementation. I don't understand why it would be desirable to step further into the validator function by default. I would have thought that would require a second s (er, actually a second and then a third), as it does in the current implementation.

What am I misunderstanding? I'm sure the problem is with my understanding here. The current behavior seems correct to me and the behavior described here strikes me as surprising.

[RafaelGSS]

I'm not sure if I understood your question. But, the idea with that is to avoid lookup internal by default when you are debugging. However, I have hit undefined behavior when enabling BlackBox through node: prefix as you can see above. When enabled it hits node:internal/* that does not look like as expected behavior since I would like to ignore ^node: which leads us to think if the prefix is working properly.

However, I have looked today at Google Chrome Node.js Debugger and looks like they removed the option to enable blackbox scripts, not sure why @nodejs/diagnostics. Besides that, I feel that we should evaluate if this feature is still good since it is a default pattern for a while.

[RafaelGSS]

Actually, the blackbox was renamed to ignore list and it has a bit different description: https://stackoverflow.com/questions/40216438/chrome-developer-tools-blackbox-option-isnt-there/44713866

[RafaelGSS]

We have discussed in the last Diagnostics WG and was defined that adding a tutorial of how to enable BlackBox script at UI and CLI will be better. Hence, I'm closing it in favor of: nodejs/diagnostics#490