vikstrous / dataloadgen

An implementation of Facebook's DataLoader in Go
MIT License
69 stars 10 forks source link

panic handling errors #14

Closed KevinJTaylor closed 5 months ago

KevinJTaylor commented 6 months ago

When implementing a "fetcher' for my dataloader, I'm not clear on what should be returned, specifically the slice of errors was causing a panic for me.

fetch func(ctx context.Context, keys []KeyT) ([]ValueT, []error)

Essentially I was doing a query which returned two elements, and was returning []items, []error{nil}. This failed as the loader logic seemed to need batch.errors[1] which did not exist of course. I changed my logic so that it returned []items, nil when successful, but what should I return if there is an error? should []error contain an entry for each of the items ...? Maybe its better to just return error and "wrap" up any child errors if they exist ...?

image
runtime/debug.Stack()
        /usr/local/opt/go/libexec/src/runtime/debug/stack.go:24 +0x5e
runtime/debug.PrintStack()
        /usr/local/opt/go/libexec/src/runtime/debug/stack.go:16 +0x13
github.com/99designs/gqlgen/graphql.DefaultRecover({0x5a0ea38?, 0x5a64a32?}, {0x6cf1380?, 0xc0003928a0?})
        /Users/kevintaylor/go/pkg/mod/github.com/99designs/gqlgen@v0.17.45/graphql/recovery.go:17 +0x6b
github.com/99designs/gqlgen/graphql.(*OperationContext).Recover(0x5a30440?, {0x6db5e98, 0xc0004f51a0}, {0x6cf1380?, 0xc0003928a0?})
        /Users/kevintaylor/go/pkg/mod/github.com/99designs/gqlgen@v0.17.45/graphql/context_operation.go:124 +0x38
kjt-gqlgen/pkg/kjtgraph.(*executionContext)._Location_processingMode.func1()
        /Users/kevintaylor/Documents/kevin/kjt-gqlgen/pkg/kjtgraph/generated.go:1060 +0x6e
panic({0x6cf1380?, 0xc0003928a0?})
        /usr/local/opt/go/libexec/src/runtime/panic.go:770 +0x132
github.com/vikstrous/dataloadgen.(*Loader[...]).LoadThunk.func1()
        /Users/kevintaylor/go/pkg/mod/github.com/vikstrous/dataloadgen@v0.0.6/dataloadgen.go:144 
vikstrous commented 5 months ago

Thanks for the report. Yeah, I'll change it to return an error about the bug in the fetch function and a best guess about what error belongs to this result.