search

graph-gophers / dataloader

Implementation of Facebook's DataLoader in Golang
MIT License
1.21k stars 75 forks source link

Protect Against Panics #14

Closed telrikk closed 7 years ago

telrikk commented 7 years ago

Hi Nick,

Can I call you Nick?

I recently noticed that panics are not caught in the call to l.batch(). The reason that this is important is because if a panic is allowed to propagate to the top of a goroutine, it will crash the process. Since dataloader spins up the goroutine, I feel like this project may be the most appropriate place to add protection against that. Please let me know if the problem and solution make sense. :)

Thanks for your time in creating and maintaining this project!

This change is Reviewable

nicksrandall commented 7 years ago

@telrikk thanks for contributing! There were some merge conflicts with another PR so I added these changes in manually. See this commit for details... https://github.com/nicksrandall/dataloader/commit/2434ed4d254f78df6ebc738410e3c8f9c568104a

telrikk commented 7 years ago

Thanks for handling the merge!:)

codecov[bot] commented 6 years ago

Codecov Report

Merging #14 into master will decrease coverage by 0.01%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #14      +/-   ##
==========================================
- Coverage   96.44%   96.42%   -0.02%     
==========================================
  Files           2        2              
  Lines         197      196       -1     
==========================================
- Hits          190      189       -1     
  Misses          7        7