Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic during data fetch is not recovered properly even with panic recovery middleware present #1

Closed
AgrimPrasad opened this issue Aug 20, 2020 · 1 comment · May be fixed by #2
Closed

Comments

@AgrimPrasad
Copy link

First of all, thanks for this library! The performance gains are very noticeable.

I found that when there is a panic during data fetch inside the handler, then the panic cannot be recovered even if there is a panic recovery middleware present in the middleware chain or if there is a recovery middleware wrapping the stampede cache handler itself.

I believe this is because the singleflight package here creates a goroutine and if a panic occurs in that goroutine, then it can never be recovered in the main goroutine.

go func() {
c.val, c.err = fn(ctx)
close(c.done)
}()

The fix should be to recover the panic properly in the callback function within HandlerWithKey and set the error properly.

stampede/http.go

Lines 82 to 95 in 39949d2

val, err := cache.GetFresh(r.Context(), key, func(ctx context.Context) (interface{}, error) {
first = true
buf := bytes.NewBuffer(nil)
ww := &responseWriter{ResponseWriter: w, tee: buf}
next.ServeHTTP(ww, r)
val := responseValue{
headers: ww.Header(),
status: ww.Status(),
body: buf.Bytes(),
}
return val, nil
})

@AgrimPrasad
Copy link
Author

@pkieltyka Is this issue fixed now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants