breezewish / express-minify

Automatically minify and cache your javascript and css files.
https://npmjs.org/package/express-minify
MIT License
86 stars 18 forks source link

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

Closed Martii closed 8 years ago

Martii commented 8 years ago

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/UglifyJS2#448

Thanks so much.

Martii commented 8 years ago

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

fabiosantoscode commented 8 years ago

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

fabiosantoscode commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

@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.

breezewish commented 8 years ago

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

Martii commented 8 years ago

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 commented 8 years ago

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

Martii commented 8 years ago

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.