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

Unexpected token errors thrown have no line/source sample in Node #3185

Open
Paril opened this issue Oct 23, 2019 · 10 comments
Open

Unexpected token errors thrown have no line/source sample in Node #3185

Paril opened this issue Oct 23, 2019 · 10 comments

Comments

@Paril
Copy link

Paril commented Oct 23, 2019

  • Rollup Version: 1.25.1
  • Operating System (or Browser): Win10
  • Node Version: v10.16.2

How Do We Reproduce?

This is fairly easy to reproduce; if you attempt to import a non-JS file with rollup (the repl can be setup to do this easily, I dunno how to link a reproduced sample from there), the line of the file is shown in the console as a plain console.log before the Error is thrown.

If you're using rollup via Node, how do I get that line sample? Not CLI, but the API. onwarn doesn't seem to be called for this. It won't tell me what file the error occurred in or anything; all I get is "Error: Unexpected token (Note that you need plugins to import files that are not JavaScript)" from the root level.

Expected Behavior

The Error includes the file that caused the Error, and source sample/line

Actual Behavior

The Error includes the Error, but nothing else. The source sample is printed to console in REPL but seems to be no way to print it in API.

@Paril
Copy link
Author

Paril commented Oct 23, 2019

Okay, I found it. Errors thrown by rollup have an undocumented "frame" member that has this info. It's only listed in the warning sections, not where errors are.

@PixievoltNo1
Copy link

I'm using Gulp integration as documented, and am getting bit by this bug.

@lukastaegert
Copy link
Member

This is not a bug in Rollup, the information is included with the error objects as stated in the comment above. If gulp integration does not show this information, an issue should be raised there.

@PixievoltNo1
Copy link

PixievoltNo1 commented Oct 29, 2019

Why would Gulp support non-standard frame and loc error object properties? Rollup is the side documenting the Gulp/Rollup integration; ensuring proper error reporting should be its job.

@shellscape
Copy link
Contributor

@PikadudeNo1 error objects often have extra information added to this. This is common practice in the node ecosystem. To assume that Rollup is not "ensuring proper error reporting" is rather naive.

@PixievoltNo1
Copy link

Rollup is currently not even documenting that its API can throw errors at all, much less how it adds additional information to those errors, or explaining how to use that info in its Gulp documentation.

@PixievoltNo1
Copy link

Which approach do you think would be better for resolving this issue - documenting errors and providing API consumers with the information needed to handle them, or making Rollup API errors more console-friendly? I'm inclined towards the latter, as I don't see much to do with errors besides logging them anyways, and I feel that should be made simple.

@lukastaegert
Copy link
Member

I tend to disagree. Not putting the error frame into the error message but keeping it separate allows for instance Rollup’s own CLI to apply special formatting to make the error more readable. This would extend to other tooling as well. Otherwise, you either keep the error as a big blob of text or manually run potentially fragile regular expressions over the message to split it again. Improving documentation, on the other hand, is always very much appreciated.

@lukastaegert
Copy link
Member

Another way forward could be to supply a custom toString method for the error that contains the frame without changing the actual message. Not sure what side-effects this would have, but this would at least allow tooling to access the raw data if needed.

@Paril
Copy link
Author

Paril commented Nov 2, 2019

If you're wanting to handle the rollup errors in a special way for your own CLI, you'd catch the error, print out the frame and then print the stack. toString() on its own should return both put together though, for the sake of those not wanting to implement their own error handler like I had to do just to print out the frame info.

You should also use a custom Error type, because it's confusing to see a built-in error type that has extra data. That's mainly what confused me, because I didn't expect TypeError to have these special properties. You should use something like RollupCompileError or similar, and document the type. That would have resolved the issue pretty quickly too, because it would have told me "they extended the type, chances are the extra info I need is inside there".

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

4 participants