Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align paths in traces #30

Open
piranna opened this issue Jul 16, 2019 · 13 comments
Open

Align paths in traces #30

piranna opened this issue Jul 16, 2019 · 13 comments

Comments

@piranna
Copy link

piranna commented Jul 16, 2019

Initially proposed at nodejs/node#22575, and cross-posting here as recommended at nodejs/node#22575 (comment).

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.

@ljharb
Copy link
Member

ljharb commented Jul 16, 2019

This proposal is currently only attempting to specify the structure, not the contents - but either way, that kind of alignment doesn't match what browsers/engines currently do, so you'd have to build your own function that used the structured data provided by this proposal in order to format the stack trace how you liked.

Alignment is a subjective thing; many people like it, but many people (myself included) find it much harder to read.

@piranna
Copy link
Author

piranna commented Jul 16, 2019

that kind of alignment doesn't match what browsers/engines currently do, so you'd have to build your own function that used the structured data provided by this proposal in order to format the stack trace how you liked.

I was suggested to implement it in Node.js and open a pull-request for that, I'll try to find some time to do it.

Alignment is a subjective thing

Agree. In my case, I find it better to have them aligned so it's easier to identify the actual functions and their location at a glance, without needing to read/scan all the stack trace lines.

@ljharb
Copy link
Member

ljharb commented Jul 16, 2019

That's a good reason to make it be a separate utility function, so that you have to opt in to this nonstandard formatting.

@piranna
Copy link
Author

piranna commented Jul 16, 2019

A wrapper function that parses and format the stack trace? Yes, doable, but not a default behaviour when an error ocurrs, that's when you need it...

@ljharb
Copy link
Member

ljharb commented Jul 16, 2019

Right - but since not everyone will prefer the same default as you, it doesn't make sense to ruin it for them and change the default. I think a wrapper function is the better approach, for node as well.

@rdking
Copy link

rdking commented Jun 22, 2021

Why not split the difference on this one? Add a formatting function to the spec. Default value can be undefined or null. If set, let the getter for Error.prototype.stack use it to format the stack entries. If it's not present, then use the engine default formatting.

@theScottyJam
Copy link

Or, if we want the paths aligned, we could just swap the order of paths and functions, that way it doesn't take up much horizontal space. It would be nice if it was easier to distinguish the stack frames from external libraries/npm modules, or even different parts of your own projects.

I guess the message shown to the end-user when an error actually goes uncaught can use a differently formatted stack trace than what's found in the "stack" property, but I doubt there would be much incentive to deviate from it.

And, I see the value in just trying to standardize what implementations are already doing, without forcing them to make changes, so while a different format would be nice, I don't really see that happening in this spec.

@mk-pmb
Copy link

mk-pmb commented Jul 1, 2021

Alignment is a subjective thing; many people like it,

And even people who like it (I do), need different kinds of whitespace in different kinds of projects, e.g.

  • just ascii space
  • full-width space
  • ascii space + non-breaking space
  • non-breaking space + ascii space
  • ascii space, dots, ascii space

@piranna
Copy link
Author

piranna commented Jul 1, 2021

And even people who like it (I do), need different kinds of whitespace in different kinds of projects, e.g.

KISS principle: use ASCII whitespaces.

@piranna
Copy link
Author

piranna commented Mar 1, 2022

How can we move this forward?

@ljharb
Copy link
Member

ljharb commented Mar 1, 2022

It’s not clear there is a path forward here, until after this proposal has landed.

@piranna
Copy link
Author

piranna commented Mar 2, 2022

until after this proposal has landed

Are you talking about my proposal of aligned stack traces, or the one of this repo about standarize them?

@ljharb
Copy link
Member

ljharb commented Mar 2, 2022

This one. Until they’re standardized as-is, it won’t be possible for any proposal to enhance or change them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants