Align paths in traces

[piranna]

When an exception is thrown or when using console.trace(), files paths are not aligned making it difficult to follow them at naked eye and specially to identify when it's one of your files, one internal module of if it's a file located inside node_modules folder:

UnhandledPromiseRejectionWarning: SyntaxError: Identifier 'body' has already been declared
     at Test.Runnable (/opt/app/node_modules/mocha/lib/runnable.js:36:25)
     at new Test (/opt/app/node_modules/mocha/lib/test.js:24:12)
     at context.it.context.specify (/opt/app/node_modules/mocha/lib/interfaces/bdd.js:85:18)
     at Function.context.it.only (/opt/app/node_modules/mocha/lib/interfaces/bdd.js:96:46)
     at Object.<anonymous> (/opt/app/src/modules/templates/template.spec.js:21:4)
     at Module._compile (internal/modules/cjs/loader.js:689:30)
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
     at Module.load (internal/modules/cjs/loader.js:599:32)
     at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
     at Function.Module._load (internal/modules/cjs/loader.js:530:3)
     at Module.require (internal/modules/cjs/loader.js:637:17)
     at require (internal/modules/cjs/helpers.js:20:18)
     at /opt/app/node_modules/mocha/lib/mocha.js:250:27
     at Array.forEach (<anonymous>)
     at Mocha.loadFiles (/opt/app/node_modules/mocha/lib/mocha.js:247:14)
     at Mocha.run (/opt/app/node_modules/mocha/lib/mocha.js:576:10)

Not sure if this is done at v8 level, but my proposal is to add spaces between the function name and the file paths so this last ones gets aligned between themselves to the longest one. In the previous trace, it would get like:

UnhandledPromiseRejectionWarning: SyntaxError: Identifier 'body' has already been declared
     at Test.Runnable                 (/opt/app/node_modules/mocha/lib/runnable.js:36:25)
     at new Test                      (/opt/app/node_modules/mocha/lib/test.js:24:12)
     at context.it.context.specify    (/opt/app/node_modules/mocha/lib/interfaces/bdd.js:85:18)
     at Function.context.it.only      (/opt/app/node_modules/mocha/lib/interfaces/bdd.js:96:46)
     at Object.<anonymous>            (/opt/app/src/modules/templates/template.spec.js:21:4)
     at Module._compile               (internal/modules/cjs/loader.js:689:30)
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
     at Module.load                   (internal/modules/cjs/loader.js:599:32)
     at tryModuleLoad                 (internal/modules/cjs/loader.js:538:12)
     at Function.Module._load         (internal/modules/cjs/loader.js:530:3)
     at Module.require                (internal/modules/cjs/loader.js:637:17)
     at require                       (internal/modules/cjs/helpers.js:20:18)
     at                                /opt/app/node_modules/mocha/lib/mocha.js:250:27
     at Array.forEach                 (<anonymous>)
     at Mocha.loadFiles               (/opt/app/node_modules/mocha/lib/mocha.js:247:14)
     at Mocha.run                     (/opt/app/node_modules/mocha/lib/mocha.js:576:10)

There would be problems if some code is parsing the trace output taking in consideration to be just only a space between the function name and the file path instead of several spaces, so this change would need to be in a major version, but anyway exception traces are not standard and such libs are very few and mostly for debuging purposses so their impact will be low, and is a small change that they would be easily added.

[addaleax]

Not sure who to talk to here. @nodejs/v8 maybe? (edit: Sorry, I thought we had a TC39 team.)

[ryzokuken]

@piranna @addaleax did we follow up on this? If not, I could start a discussion on the TC39 mailing list and CC you.

[piranna]

No, I didn't. Please start the discussion at TC39. Don't know if it's of their competence or v8 or Node.js, but I think if so TC39 would prefer instead to define a standard traces spec... At least this change would be a start for that :-)

[hashseed]

You could easily achieve this by overriding Error.prepareStackTrace. I don't think we want to add the complexity for this in V8, especially since this is very bikesheddy.

[piranna]

Maybe would makes sense to implement this here in Node.js? Later it could be used as a proof-of-concept to implement it in other environments... Would you accept a pull-request for this?

[devsnek]

@piranna i'm in the middle of rewriting our error stack decoration over in #23926. after that lands you could open a pr modifying the new stack decoration method we use.

fair warning though, i don't think many people will be partial to the format you're proposing. most people don't have enough screen real-estate for it to be practical. this seems like its better suited to happen in userland with Error.prepareStackTrace, as hashseed said.

There is also https://github.com/tc39/proposal-error-stacks

[piranna]

Now that #23926 has landed in master, can we start moving this? :-) Where should we start?

[srl295]

@piranna just triaging here…

• it might be good to open an issue on https://github.com/tc39/proposal-error-stacks referencing this.
• "you could open a pr modifying the new stack decoration method we use."

[piranna]

• it might be good to open an issue on tc39/proposal-error-stacks referencing this.

Done at tc39/proposal-error-stacks#30.

• "you could open a pr modifying the new stack decoration method we use."

Are you propossing that I implement it on Node.js code and create a pull-request with my changes?

[github-actions]

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

[piranna]

How can we move this forward? Maybe a PR implementing it here? I was asked to open one on tc-39 and got already stalled there...