NickNaso / ghostscript4js

Ghostscript4JS binds the Ghostscript C API to the Node.JS world.
http://www.nacios.it
Apache License 2.0
66 stars 19 forks source link

Fix exception handling #40

Closed lorenyu closed 5 years ago

lorenyu commented 5 years ago

Context

In ghostscript4js, whenever the gs command returns a nonzero exit code it triggers a fatal unrecoverable error that can't be handled in JavaScript. In particular, the GhostscriptManager class tries to throw a char* exception and the GhostscriptWorker (subclass of Napi::AsyncWorker) tries to catch it to set the JavaScript error with SetError. While I'm not deeply familiar with Napi, I've found that if we throw a std::string exception instead and change SetError to set the string of the exception rather than use Napi::String::New, we can properly handle the error in JavaScript.

Changes

Incidental change

Testing

NickNaso commented 5 years ago

Hi @lorenyu I saw the PR only now. Why did you close it?

lorenyu commented 5 years ago

@NickNaso sorry that was a misfire. I hadn't finished with it yet, wanted to add a unit test to cover the case. I put up a new PR with the same fix and the unit test #41, can you review that one? Thanks!

NickNaso commented 5 years ago

@lorenyu I'm very happy about your contribution. When you will end the work open a new PR and I will review it.

lorenyu commented 5 years ago

@NickNaso happy to contribute! I finished the work and opened up PR #41, let me know if there is anything else you need from me there. :)