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

Try/catch around pcbStackup function #1

Open
kasbah opened this issue Apr 24, 2018 · 4 comments
Open

Try/catch around pcbStackup function #1

kasbah opened this issue Apr 24, 2018 · 4 comments

Comments

@kasbah
Copy link
Contributor

kasbah commented Apr 24, 2018

Relating to errors like mcous/gerber-plotter#14, I think we need to try {} catch e {reject(e)} around pcbStackup here in case it does crash.

@mcous
Copy link

mcous commented Apr 24, 2018

Given that gerber-plotter operates asynchronously in a stream, I don't think try-catch will work. Just a hunch, but heads up

@ju5t
Copy link
Contributor

ju5t commented Apr 24, 2018

It doesn't catch the error from mcous/gerber-plotter#14 but it's still useful.

ju5t added a commit that referenced this issue Apr 24, 2018
@kasbah
Copy link
Contributor Author

kasbah commented Apr 24, 2018

I was thinking about it a bit more after I opened this. I think it would reject the promise on a normal error anyway.

@mcous, maybe something could be done in gerber-to-svg or dependencies to better recover from errors within the stream processing?

@ju5t
Copy link
Contributor

ju5t commented Apr 24, 2018

I was thinking about it a bit more after I opened this. I think it would reject the promise on a normal error anyway.

Arguably the current try/catch-block is a bit large. It doesn't do much harm at the moment though so I believe it can stay as it is. Additional error handling can be added when they are found and not caught in the current solution.

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

No branches or pull requests

3 participants