Skip to content
This repository has been archived by the owner on Nov 20, 2020. It is now read-only.

NFR: catch callbacks, emitters, or equivalent options #44

Closed
Martii opened this issue Jan 20, 2016 · 9 comments
Closed

NFR: catch callbacks, emitters, or equivalent options #44

Martii opened this issue Jan 20, 2016 · 9 comments
Labels

Comments

@Martii
Copy link

Martii commented Jan 20, 2016

In order to easily provide some data back to UglifyJS2 the current code "ignores" any errors.

It would be very helpful to them to have us be able to provide this data to them and we could attach our own handler to the error to track down which user script request (code in general) is causing an error being thrown.

I can of course hack in a stderr message in a fork but thought something more elegant might be available.

Applies to mishoo/UglifyJS#448

Thanks so much.

@Martii
Copy link
Author

Martii commented Jan 20, 2016

Thank ya... won't have start time to integrate this until tomorrow pending any corporeal duties that I need to attend to first. :)

@fabiosantoscode
Copy link

Afaik uglifyjs errors are thrown. Using a simple try.. catch you should be able to catch and take note of the error, and line/col number.

Am I misunderstanding the issue? :D

1 similar comment
@fabiosantoscode
Copy link

Afaik uglifyjs errors are thrown. Using a simple try.. catch you should be able to catch and take note of the error, and line/col number.

Am I misunderstanding the issue? :D

@Martii
Copy link
Author

Martii commented Jan 20, 2016

@fabiosantoscode
Since we are presentational userscripts... only one copy is held on our site... we can mirror the line number, as you said, but the source code could change between the time I read the log, report it if needed, have you examine it, etc... I'll do my best to catch them as they happen but there is no guarantee that someone won't upload and install a new master version within 60 seconds... e.g. I can report the error but not the full source code (known as body in the parameter list... it's just too big to log... could have up to a 1MiB dump each time)... unless the error object contains which specific JavaScript code line content is failing.

@Martii
Copy link
Author

Martii commented Jan 21, 2016

@fabiosantoscode
Here's an example output based off SummerWish's test.js test (made this pretty btw)

message: Unterminated multiline comment
line: 1
col: 0
pos: 0
stack: Error
    at new JS_Parse_Error (eval at <anonymous> (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:1778:18)
    at js_error (eval at <anonymous> (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:1786:11)
    at parse_error (eval at <anonymous> (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:1899:9)
    at eval (eval at <anonymous> (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:2141:36)
    at handle_slash (eval at <anonymous> (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:2098:20)
    at Object.next_token [as input] (eval at <anonymous> (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:2168:27)
    at next (eval at <anonymous> (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:2285:25)
    at Object.parse (eval at <anonymous> (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/uglify-js/tools/node.js:22:1), <anonymous>:2271:15)
    at ~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/uglify-js/tools/node.js:61:33
    at Array.forEach (native)
    at Object.exports.minify (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/uglify-js/tools/node.js:56:15)
    at Minifier.compileAndMinify (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/express-minify/minifier.js:58:32)
    at ~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/express-minify/index.js:165:20
    at MemoryCache.get (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/express-minify/cache.js:38:5)
    at ServerResponse.res.end (~/repo/git/OpenUserJS.org/martii/OpenUserJS.org/node_modules/express-minify/index.js:162:19)
    at PassThrough.onend (_stream_readable.js:490:10)
    at PassThrough.g (events.js:260:16)
    at emitNone (events.js:72:20)
    at PassThrough.emit (events.js:166:7)
    at endReadableNT (_stream_readable.js:905:12)
    at doNTCallback2 (node.js:441:9)
    at process._tickDomainCallback (node.js:396:17)

... library source of (body) ...

/* this is a broken JavaScript!

var test = 10;

... this particular error I wouldn't want to send to you if it was just a mistake of not closing the comment.

@summerwish
I'm going to end up moving the console.log to a console.warn ... don't know if that was a conscious decision you made to do stdout as a default.

Very kewl and kuick addition btw. :) Thanks again.

Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Jan 21, 2016
* Add in a basic handler for compiling/minification errors and output to stdout with *express-minify* ... not doing `stack` as that consumes a bit.. and `body` can be up to 1MiB... so that will never get logged. Feature request of breezewish/express-minify#44
* *compression* is just dep updates according to their changelog

**NOTE**:
* This can still use some TLC but hacking it in just so we get some of this data to start

Also applies to OpenUserJS#432 and very loosely OpenUserJS#53
@breezewish
Copy link
Owner

Are you referring to this? You can write anything you want there :-) write logs, etc.

@Martii
Copy link
Author

Martii commented Jan 21, 2016

Yes... but was wondering why you chose stdout as the default for an error trap instead of stderr (console.warn and console.error are typically used for stderr)... no biggie as I'm overriding it anyhow... just curious.

@breezewish
Copy link
Owner

You are right. I just didn't consider about this at that time LOL

@Martii
Copy link
Author

Martii commented Jan 21, 2016

No problem... I'm going to need to dig into how I'm going to look up the related script too... I don't have access to the request I think... so not entirely sure how other than a reverse lookup which will be cpu intensive. I've asked sizzle to assist to find an option... at least we'll be able to see if there is an issue but we won't really know where just yet.

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

No branches or pull requests

3 participants