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

RecoveryWithWriter takes mulitple RecoveryFuncs but only applies the first #4044

Open
nesselchen opened this issue Aug 30, 2024 · 3 comments

Comments

@nesselchen
Copy link

Description

RecoveryWithWritter has signature func(w io.Writer, recovery ...gin.RecoveryFunc) but only applies the first func from recovery.
If no RecoveryFuncs were passed, it uses the defaultHandleRecovery func.

If I want to register multiple RecoveryFuncs, it would seem plausible to apply them all at once, but with this implementation most would get discarded silently. Additionally, this results in a panicking request returning code 200 instead of the pre-configured code 500.

I don't see why it couldn't allow multiple RecoveryFuncs with each handling the error separately. Alternatively I would appreciate it if the documentation gave hints to this being the intended behavior.

How to reproduce

Here's the excerpt from the gin package (recover.go:43)

// RecoveryWithWriter returns a middleware for a given writer that recovers from any panics and writes a 500 if there was one.
func RecoveryWithWriter(out io.Writer, recovery ...RecoveryFunc) HandlerFunc {
	if len(recovery) > 0 {
		return CustomRecoveryWithWriter(out, recovery[0])
	}
	return CustomRecoveryWithWriter(out, defaultHandleRecovery)
}

Expectations

func PrintErr(_ *gin.Context, err any) {
  if err != nil {
    fmt.Println(err)
  }
}

func Abort(ctx *gin.Context, _ any) {
  ctx.AbortWithStatus(500)
}

// ...

router.Use(RecoveryWithWriter(logger, PrintErr, Abort)) // -> should print the err and abort the request

## Actual result

Under the current implementation only the error would get logged, with the request still returning 200 after recovering.

## Environment

- go version: go1.22.6
- gin version (or commit ref):	github.com/gin-gonic/gin v1.9.1
- operating system: macOS
@JimChenWYU
Copy link

I think it's not support mulitple RecoveryFuncs but provides optional parameters.

@nesselchen
Copy link
Author

Yeah, that's what I assumed. Couldn't you put that in the documentation to limit the risk? Either that or changing it to allow multiple RecoveryFuncs. I still don't really see why since they don't interfere with each other.

@RedCrazyGhost
Copy link
Contributor

4cabdd3 The last submission has been 4 years ago, and this part may involve problems with the gin execution stack, which has not been processed

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

No branches or pull requests

3 participants