opened
Andarist
username_0
Hi 👋

I'd like to discuss what's the best way to report errors. I.e. I got such [report](https://github.com/rollup/rollup-plugin-babel/issues/216), but without digging into the mechanics I'm not sure how I can improve it. Would be super useful to have some kind of docs for plugin authors - especially that I think there were some changes lately to the exposed APIs, new hooks etc.

I also had a similar situation reported in other issue. I've tracked it down to rollup catching error thrown by babel and swallowing it's stack trace which made debugging it longer and issue being raised in wrong repository. Is there a way to preserve stack trace of the original error?
[question] plugin error reporting
issue
created
tivac
username_1
Check out the "Plugin Context" section of https://github.com/rollup/rollup/wiki/Plugins#creating-plugins

Sounds like you want to be using `this.warn()` or `this.error()`.
comment
created
guybedford
username_2
@username_0 thanks for digging into this... YES it sounds like something worth checking! In plugins we encourage `this.error` being called in the transform hook as a whole bunch of contextual info gets attached - https://github.com/rollup/rollup/blob/master/src/utils/transform.ts#L79. If there is anything that can be done to neaten up these workflows though please by all means look into it further!
comment
created
Andarist
username_0
@username_2 I've tinkered a little bit with it and I have some findings:
- thrown `frame` should always be respected, fixing this in https://github.com/rollup/rollup/pull/2309
- slight problem with babel is that it puts `frame` into `err.message` - maybe I will be able to do smth about it on babel's side, we should be able to at least put it on `err` as extra property too (this is why frame is visible twice for errors thrown by babel when tranforming with rollup-plugin-babel)
- while `err.message` gets a nice red color in the CLI, frame doesnt - and as it being often extremely helpful I would advocate for colouring it too
- kinda same thing with filename/id and pos - but not only that message becomes invisible for people because it's not coloured, it also lacks any context. I see a nice opportunity here for formatting the WHOLE thing better for humans. It might be a good idea to colour whole output the same way, but also you could report something like:
```
Error thrown while transforming "path/to/src/file.js:3:11":

[!] (babel plugin) SyntaxError: /test__/test.js: Unexpected token, expected ";"

<CODE_FRAME_PLACEHOLDER>
```
- also the stack trace of the error is way too important to just swallow it, I know most people won't follow it and might find it obscurring the output, but it often might be vital for finding a problem in underlaying tool - I would suggest adding it to the output (this one might have always black/grey color, as it's less important for "most" people)
comment
created
lukastaegert
username_3
Definitely! I like the idea of adding information about in which step the error was thrown. Of course with the current architecture, this would mean adding another property to the error object, maybe some text snipped.

I am not so fond of the idea of separating the file name and the code frame. For rollup's own error, the current order makes the file name a heading for the code frame which gives it more context:
```
[!] (funny plugin) SyntaxError: Some evil error
src/main.js: (3:12)
1: import './dep.js';
2: 
3: console.log(x);
               ^
```

Tearing them apart could make them seem unrelated (In an unrelated note: For warnings, the file name is bold while for errors, it is not. That should certainly be aligned)

But maybe there is no harm in repeating the file name for errors, which could make it look like this (we can really play around a little here):
```
[!] SyntaxError while transforming "src/main.js" (in funny plugin):

Some really terrible error

src/main.js: (3:12)
1: import './dep.js';
2: 
3: console.log(x);
               ^
SyntaxError: Some really terrible error
    at error (/x/y/z.js:6:14)
    at Object.error (/a/b/c.js:89:4)
```
comment
created
Andarist
username_0
It could, I'm wondering though if it doesn't warrant for a change in babel - exposing `.frame` on thrown errors + assembling a nice error output in cli module rather than directly in `err.message`. OTOH some tools could swallow nice `.frame`s and return worse errors to the caller. Either way I need to prepare a PR exposing `.frame` as it would made clean up easier.

As to the rest - thanks for your thoughts, gonna probably look into improving this some time soon (hopefully 😉 )
comment
created
guybedford
username_2
@username_0 would value your feedback on #2382 if you can take a look.
comment
created
shellscape
username_4
Hey folks. This is a saved-form message, but rest assured we mean every word. The Rollup team is attempting to clean up the Issues backlog in the hopes that the active and still-needed, still-relevant issues bubble up to the surface. With that, we're closing issues that have been open for an eon or two, and have gone stale like pirate hard-tack without activity.

We really appreciate the folks have taken the time to open and comment on this issue. Please don't confuse this closure with us not caring or dismissing your issue, feature request, discussion, or report. The issue will still be here, just in a closed state. If the issue pertains to a bug, please re-test for the bug on the latest version of Rollup and if present, please tag @username_4 and request a re-open, and we'll be happy to oblige.
comment
closed
shellscape
username_4
issue