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

Deno creating huge error messages from auto-generated JS #4255

Closed
Sharpiro opened this issue Mar 5, 2020 · 3 comments · Fixed by #4487
Closed

Deno creating huge error messages from auto-generated JS #4255

Sharpiro opened this issue Mar 5, 2020 · 3 comments · Fixed by #4487
Labels
bug Something isn't working correctly

Comments

@Sharpiro
Copy link

Sharpiro commented Mar 5, 2020

deno 0.35.0
v8 8.1.310
typescript 3.8.2

I was working with wabt.js today and trying to get it working with deno as well as the browser. When testing, I noticed deno was creating huge errors, so big in fact, I had to save them to a file to read them. It almost seems like the error contains the entire program, or one of the huge lines. wabt.js is built using emscripten, which creates pretty ugly javascript, so I'm not sure if this only occurs when deno runs something that emscripten has built.

An example of the error:

deno https://raw.githubusercontent.com/Sharpiro/bugs/0d41bbe0d3ee8cf2e052cbc76d7f00b917a933fd/deno/large_errors/index.js

add &> temp to output it to file

deno https://raw.githubusercontent.com/Sharpiro/bugs/0d41bbe0d3ee8cf2e052cbc76d7f00b917a933fd/deno/large_errors/index.js &> temp

As an aside here is a fork of WABT I got working today in deno after fixing the errors:
https://github.com/Sharpiro/wabt.js/blob/9ea629b1c31f80c70efdf9d01b489274beb0ddb1/index.js

@kevinkassimo
Copy link
Contributor

kevinkassimo commented Mar 5, 2020

We should restrict the error line to be shorter than e.g. 150 characters IMO

@bartlomieju bartlomieju added the bug Something isn't working correctly label Mar 5, 2020
@kevinkassimo
Copy link
Contributor

kevinkassimo commented Mar 24, 2020

So I have a new idea: maybe we should just show nothing if the error line is too long, since it is not likely going to help anyways. Also truncation has the problem of unicode substring handling, and I am reluctant to introduce a new crate like unicode-segmentation just to deal with grapheme clusters

@kitsonk
Copy link
Contributor

kitsonk commented Mar 24, 2020

Given the problem with truncation (though I suspect that would be an edge case and if you mis truncated a line, we are talking about one char being mangled), I still think if the line is longer than say 120 or 150, it just doesn't make sense to output it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants