Skip to content
This repository has been archived by the owner on Dec 5, 2019. It is now read-only.

fix(uglify/worker): better {Error} handling #244

Closed

Conversation

alexander-akait
Copy link
Member

No description provided.

@michael-ciniawsky
Copy link
Member

Could you please show a screenshot of the output ? Nice if it already works with this elegant change :)

@michael-ciniawsky michael-ciniawsky added this to the 1.2.2 milestone Feb 26, 2018
@michael-ciniawsky
Copy link
Member

Also the {Error} shape/type of the err before pushing to compilation.errors needs to be checked :)

@alexander-akait
Copy link
Member Author

@michael-ciniawsky what do you mean? 😄

@michael-ciniawsky
Copy link
Member

A screenshot/logs of the {Error} and what does the worker return as err {String|Object} {Object} === instanceof Error (when it is passed to compilation.errors.push(err)) ? 🙃

@michael-ciniawsky
Copy link
Member

Could we clearify if this definitely fixes the {Error} handling and get this PR and #243 out asap ? :)

@alexander-akait
Copy link
Member Author

@michael-ciniawsky 👍

@alexander-akait alexander-akait force-pushed the fix-uglify-worker-better-error-handling branch from 47afba4 to ba16f72 Compare March 2, 2018 11:08
@alexander-akait alexander-akait force-pushed the fix-uglify-worker-better-error-handling branch from ba16f72 to 376aedf Compare March 2, 2018 11:10
@alexander-akait
Copy link
Member Author

alexander-akait commented Mar 2, 2018

@michael-ciniawsky this solution don't works because we can't build UglifyJsPlugin.buildError (for right source map position and other meta information) we should serialize error for right handle 😞 It is only right way here

@michael-ciniawsky
Copy link
Member

michael-ciniawsky commented Mar 7, 2018

Can you explain this a bit more detailed please :) ? The buildUglifyError could maybe made it's own{Error} Class instead, which handles the serialized error {String|Object}

Uglify[Worker]Error.js

class Uglify[Worker]Error extends Error {
   constructor (serialized) {
      const err = JSON.parse(serialized)
       
      this.name = err.name
      this.message = `\n\n(${err.line}:${err.column}) ${err.message}...\n`
      
      Error.captureStackTrace(this, this.constructor)
   }
}

module.exports = Uglify[Worker]Error

@joshwiens
Copy link
Member

Please rebase with current master

@alexander-akait alexander-akait modified the milestones: 1.2.6, 2.0.0 Jul 3, 2018
@michael-ciniawsky
Copy link
Member

@evilebottnawi Status? :)

@alexander-akait
Copy link
Member Author

@michael-ciniawsky apparently something was wrongly installed or configured, the problem is no longer there

@alexander-akait alexander-akait deleted the fix-uglify-worker-better-error-handling branch August 14, 2018 10:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants