janpfeifer / gonb

GoNB, a Go Notebook Kernel for Jupyter
https://github.com/janpfeifer/gonb
MIT License
469 stars 32 forks source link

Error handling #48

Closed bagel897 closed 11 months ago

bagel897 commented 11 months ago

The problem

When gonb runs into errors, it generates HTML error reports. These are great for a jupyter notebook, but don't render in nbmake

The solution

Add an option to print out raw error messages instead of html reports, set by a command line flag. This works inside nbmake. image image

janpfeifer commented 11 months ago

Thanks @bagel897 , this looks really great! Having it work well nbmake facilitates testing -- I have to start using, it also for GoNB (lots of things I didn't test because were not easy to).

A few suggestions (all very small "nit-picks"), I hope you don't mind.

Let me know if you would like to change them before merging (or if you disagree), or I'll just merge your PR as is do the small changes later (likely on Monday morning)

cheers, and thanks!!

bagel897 commented 11 months ago
  1. Sounds good!
  2. errorcontext.go still has the HTML template. But I understand combining the two files, it'd make it simpler. Alternatively, I can move the HTML report part back into errorcontext.go
  3. Ill do that as well

I'll do the changes monday morning, is that alright?

janpfeifer commented 11 months ago

That's perfect, thanks again Ellen.

On Sat, Aug 12, 2023, 08:24 Ellen Agarwal @.***> wrote:

  1. Sounds good!
  2. errorcontext.go still has the HTML template. But I understand combining the two files, it'd make it simpler. Alternatively, I can move the HTML report part back into errorcontext.go
  3. Ill do that as well

I'll do the changes monday morning, is that alright?

— Reply to this email directly, view it on GitHub https://github.com/janpfeifer/gonb/pull/48#issuecomment-1675737521, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABY5KE7BY7LA3VKE2LEWAGDXU4OSZANCNFSM6AAAAAA3NKM4H4 . You are receiving this because you commented.Message ID: @.***>

bagel897 commented 11 months ago

Thanks for the feedback!

  1. Done
  2. I renamed it to htmlerror and moved all the HTML code back into the file. This leaves it over 100 lines, which should be good. Alternatively, we could move all the error files into their own error module, would that work better?
  3. I updated the help flag

Lmk if you have any other suggestions