From 384c40a139aa564c94a57295d1ddd61c886ecac4 Mon Sep 17 00:00:00 2001 From: Evan Shaw Date: Sun, 11 Aug 2019 21:22:28 +1200 Subject: [PATCH 1/4] Recover from panics that happen during fetch This also adds a new optional Recover field to Config structs, to allow control over transforming recovered values into errors. When it's nil, fmt.Errorf("%v", v) is used. --- pkg/generator/template.go | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/pkg/generator/template.go b/pkg/generator/template.go index 48f5ba2..92bac87 100644 --- a/pkg/generator/template.go +++ b/pkg/generator/template.go @@ -12,6 +12,7 @@ var tpl = template.Must(template.New("generated"). package {{.Package}} import ( + "fmt" "sync" "time" @@ -21,7 +22,7 @@ import ( // {{.Name}}Config captures the config to create a new {{.Name}} type {{.Name}}Config struct { - // Fetch is a method that provides the data for the loader + // Fetch is a method that provides the data for the loader Fetch func(keys []{{.KeyType.String}}) ([]{{.ValType.String}}, []error) // Wait is how long wait before sending a batch @@ -29,6 +30,10 @@ type {{.Name}}Config struct { // MaxBatch will limit the maximum number of keys to send in one batch, 0 = not limit MaxBatch int + + // Recover is a function to transform a recovered value into an error. + // If a function is not supplied, the value is formatted with fmt.Errorf("%v", v). + Recover func(v interface{}) error } // New{{.Name}} creates a new {{.Name}} given a fetch, wait, and maxBatch @@ -37,10 +42,11 @@ func New{{.Name}}(config {{.Name}}Config) *{{.Name}} { fetch: config.Fetch, wait: config.Wait, maxBatch: config.MaxBatch, + recover: config.Recover, } } -// {{.Name}} batches and caches requests +// {{.Name}} batches and caches requests type {{.Name}} struct { // this method provides the data for the loader fetch func(keys []{{.KeyType.String}}) ([]{{.ValType.String}}, []error) @@ -51,6 +57,9 @@ type {{.Name}} struct { // this will limit the maximum number of keys to send in one batch, 0 = no limit maxBatch int + // this transforms recovered panic values into errors + recover func(v interface{}) error + // INTERNAL // lazily created cache @@ -239,6 +248,15 @@ func (b *{{.Name|lcFirst}}Batch) startTimer(l *{{.Name}}) { } func (b *{{.Name|lcFirst}}Batch) end(l *{{.Name}}) { + defer func() { + if r := recover(); r != nil { + if l.recover != nil { + b.error = []error{l.recover(r)} + } else { + b.error = []error{fmt.Errorf("%v", r)} + } + } + }() b.data, b.error = l.fetch(b.keys) close(b.done) } From 5058b1d8a20460c977fd80937ca1b3acfd8082db Mon Sep 17 00:00:00 2001 From: Evan Shaw Date: Sun, 11 Aug 2019 21:24:30 +1200 Subject: [PATCH 2/4] Regenerate examples --- example/pkgname/userloader_gen.go | 18 ++++++++++++++++++ example/slice/usersliceloader_gen.go | 18 ++++++++++++++++++ example/userloader_gen.go | 18 ++++++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/example/pkgname/userloader_gen.go b/example/pkgname/userloader_gen.go index 3495d73..2106d82 100644 --- a/example/pkgname/userloader_gen.go +++ b/example/pkgname/userloader_gen.go @@ -3,6 +3,7 @@ package differentpkg import ( + "fmt" "sync" "time" @@ -19,6 +20,10 @@ type UserLoaderConfig struct { // MaxBatch will limit the maximum number of keys to send in one batch, 0 = not limit MaxBatch int + + // Recover is a function to transform a recovered value into an error. + // If a function is not supplied, the value is formatted with fmt.Errorf("%v", v). + Recover func(v interface{}) error } // NewUserLoader creates a new UserLoader given a fetch, wait, and maxBatch @@ -27,6 +32,7 @@ func NewUserLoader(config UserLoaderConfig) *UserLoader { fetch: config.Fetch, wait: config.Wait, maxBatch: config.MaxBatch, + recover: config.Recover, } } @@ -41,6 +47,9 @@ type UserLoader struct { // this will limit the maximum number of keys to send in one batch, 0 = no limit maxBatch int + // this transforms recovered panic values into errors + recover func(v interface{}) error + // INTERNAL // lazily created cache @@ -219,6 +228,15 @@ func (b *userLoaderBatch) startTimer(l *UserLoader) { } func (b *userLoaderBatch) end(l *UserLoader) { + defer func() { + if r := recover(); r != nil { + if l.recover != nil { + b.error = []error{l.recover(r)} + } else { + b.error = []error{fmt.Errorf("%v", r)} + } + } + }() b.data, b.error = l.fetch(b.keys) close(b.done) } diff --git a/example/slice/usersliceloader_gen.go b/example/slice/usersliceloader_gen.go index c2d6e83..ebd55a2 100644 --- a/example/slice/usersliceloader_gen.go +++ b/example/slice/usersliceloader_gen.go @@ -3,6 +3,7 @@ package slice import ( + "fmt" "sync" "time" @@ -19,6 +20,10 @@ type UserSliceLoaderConfig struct { // MaxBatch will limit the maximum number of keys to send in one batch, 0 = not limit MaxBatch int + + // Recover is a function to transform a recovered value into an error. + // If a function is not supplied, the value is formatted with fmt.Errorf("%v", v). + Recover func(v interface{}) error } // NewUserSliceLoader creates a new UserSliceLoader given a fetch, wait, and maxBatch @@ -27,6 +32,7 @@ func NewUserSliceLoader(config UserSliceLoaderConfig) *UserSliceLoader { fetch: config.Fetch, wait: config.Wait, maxBatch: config.MaxBatch, + recover: config.Recover, } } @@ -41,6 +47,9 @@ type UserSliceLoader struct { // this will limit the maximum number of keys to send in one batch, 0 = no limit maxBatch int + // this transforms recovered panic values into errors + recover func(v interface{}) error + // INTERNAL // lazily created cache @@ -220,6 +229,15 @@ func (b *userSliceLoaderBatch) startTimer(l *UserSliceLoader) { } func (b *userSliceLoaderBatch) end(l *UserSliceLoader) { + defer func() { + if r := recover(); r != nil { + if l.recover != nil { + b.error = []error{l.recover(r)} + } else { + b.error = []error{fmt.Errorf("%v", r)} + } + } + }() b.data, b.error = l.fetch(b.keys) close(b.done) } diff --git a/example/userloader_gen.go b/example/userloader_gen.go index 470ba6a..c98f1b0 100644 --- a/example/userloader_gen.go +++ b/example/userloader_gen.go @@ -3,6 +3,7 @@ package example import ( + "fmt" "sync" "time" ) @@ -17,6 +18,10 @@ type UserLoaderConfig struct { // MaxBatch will limit the maximum number of keys to send in one batch, 0 = not limit MaxBatch int + + // Recover is a function to transform a recovered value into an error. + // If a function is not supplied, the value is formatted with fmt.Errorf("%v", v). + Recover func(v interface{}) error } // NewUserLoader creates a new UserLoader given a fetch, wait, and maxBatch @@ -25,6 +30,7 @@ func NewUserLoader(config UserLoaderConfig) *UserLoader { fetch: config.Fetch, wait: config.Wait, maxBatch: config.MaxBatch, + recover: config.Recover, } } @@ -39,6 +45,9 @@ type UserLoader struct { // this will limit the maximum number of keys to send in one batch, 0 = no limit maxBatch int + // this transforms recovered panic values into errors + recover func(v interface{}) error + // INTERNAL // lazily created cache @@ -217,6 +226,15 @@ func (b *userLoaderBatch) startTimer(l *UserLoader) { } func (b *userLoaderBatch) end(l *UserLoader) { + defer func() { + if r := recover(); r != nil { + if l.recover != nil { + b.error = []error{l.recover(r)} + } else { + b.error = []error{fmt.Errorf("%v", r)} + } + } + }() b.data, b.error = l.fetch(b.keys) close(b.done) } From 3a29fb650332a16008f14d3ae8ef7e710398f6ae Mon Sep 17 00:00:00 2001 From: Evan Shaw Date: Mon, 12 Aug 2019 20:48:34 +1200 Subject: [PATCH 3/4] Tab -> spaces --- pkg/generator/template.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/generator/template.go b/pkg/generator/template.go index 92bac87..c0d69b7 100644 --- a/pkg/generator/template.go +++ b/pkg/generator/template.go @@ -12,7 +12,7 @@ var tpl = template.Must(template.New("generated"). package {{.Package}} import ( - "fmt" + "fmt" "sync" "time" From 53233eb3eda46d14b2f8428c40733bfc4869c7d8 Mon Sep 17 00:00:00 2001 From: Evan Shaw Date: Mon, 12 Aug 2019 21:18:35 +1200 Subject: [PATCH 4/4] Add tests, fix bug --- example/pkgname/userloader_gen.go | 2 +- example/slice/usersliceloader_gen.go | 2 +- example/user_test.go | 20 ++++++++++++++++++++ example/userloader_gen.go | 2 +- pkg/generator/template.go | 2 +- 5 files changed, 24 insertions(+), 4 deletions(-) diff --git a/example/pkgname/userloader_gen.go b/example/pkgname/userloader_gen.go index 2106d82..72b3f38 100644 --- a/example/pkgname/userloader_gen.go +++ b/example/pkgname/userloader_gen.go @@ -236,7 +236,7 @@ func (b *userLoaderBatch) end(l *UserLoader) { b.error = []error{fmt.Errorf("%v", r)} } } + close(b.done) }() b.data, b.error = l.fetch(b.keys) - close(b.done) } diff --git a/example/slice/usersliceloader_gen.go b/example/slice/usersliceloader_gen.go index ebd55a2..56df6ae 100644 --- a/example/slice/usersliceloader_gen.go +++ b/example/slice/usersliceloader_gen.go @@ -237,7 +237,7 @@ func (b *userSliceLoaderBatch) end(l *UserSliceLoader) { b.error = []error{fmt.Errorf("%v", r)} } } + close(b.done) }() b.data, b.error = l.fetch(b.keys) - close(b.done) } diff --git a/example/user_test.go b/example/user_test.go index 342a4ad..b390361 100644 --- a/example/user_test.go +++ b/example/user_test.go @@ -1,6 +1,7 @@ package example import ( + "errors" "fmt" "strings" "sync" @@ -29,6 +30,8 @@ func TestUserLoader(t *testing.T) { for i, key := range keys { if strings.HasPrefix(key, "E") { errors[i] = fmt.Errorf("user not found") + } else if strings.HasPrefix(key, "P") { + panic("something bad happened") } else { users[i] = &User{ID: key, Name: "user " + key} } @@ -193,4 +196,21 @@ func TestUserLoader(t *testing.T) { require.Error(t, err2[1]) require.Equal(t, "user U6", users2[0].Name) }) + + t.Run("fetch panic with recover func", func(t *testing.T) { + expectedErr := errors.New("transformed") + dl.recover = func(interface{}) error { + return expectedErr + } + u, err := dl.Load("P1") + require.Nil(t, u) + require.Equal(t, err, expectedErr) + dl.recover = nil + }) + + t.Run("fetch panic with no recover func", func(t *testing.T) { + u, err := dl.Load("P1") + require.Nil(t, u) + require.Error(t, err) + }) } diff --git a/example/userloader_gen.go b/example/userloader_gen.go index c98f1b0..42bb1e9 100644 --- a/example/userloader_gen.go +++ b/example/userloader_gen.go @@ -234,7 +234,7 @@ func (b *userLoaderBatch) end(l *UserLoader) { b.error = []error{fmt.Errorf("%v", r)} } } + close(b.done) }() b.data, b.error = l.fetch(b.keys) - close(b.done) } diff --git a/pkg/generator/template.go b/pkg/generator/template.go index c0d69b7..c0cc38f 100644 --- a/pkg/generator/template.go +++ b/pkg/generator/template.go @@ -256,8 +256,8 @@ func (b *{{.Name|lcFirst}}Batch) end(l *{{.Name}}) { b.error = []error{fmt.Errorf("%v", r)} } } + close(b.done) }() b.data, b.error = l.fetch(b.keys) - close(b.done) } `))