search

graph-gophers / dataloader

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

Handle incorrect BatchFunc behavior #5

Closed tonyghita closed 7 years ago

tonyghita commented 7 years ago

Currently there is no check that the number of items returned from a user-supplied batch function is the same as the number of requests.

This causes an uncaught panic when it occurs. The reference implementation throws an informative error in this case (https://github.com/facebook/dataloader/blob/master/src/index.js#L271-L280).

I wrote a failing test case to show the issue, but I'm not sure as to the best solution here.

This change is Reviewable

tonyghita commented 7 years ago

I took a shot at a possible resolution that fixes the panics in 9574d86. Let me know what you think @nicksrandall.

nicksrandall commented 7 years ago

I had almost that exact same code in there before but I pulled it out. I can see how it would be a hard issue to hunt down so I'm ok with adding it back in.