commit 2358cf4e433401e9a42336f056df1ab223f89057
parent 20978b1278648c70e4fbeb8ebaf216f3b743519d
Author: tobi <31960611+tsmethurst@users.noreply.github.com>
Date: Fri, 2 Jun 2023 15:19:43 +0200
[bugfix] Overwrite API client closed errors with `499 - Client Closed Request` (#1857)
* [bugfix] Overwrite client closed errors with 499
* bleep bloop
* review changes
Diffstat:
3 files changed, 69 insertions(+), 12 deletions(-)
diff --git a/internal/api/util/errorhandling.go b/internal/api/util/errorhandling.go
@@ -83,25 +83,44 @@ func genericErrorHandler(c *gin.Context, instanceGet func(ctx context.Context) (
}
}
-// ErrorHandler takes the provided gin context and errWithCode and tries to serve
-// a helpful error to the caller. It will do content negotiation to figure out if
-// the caller prefers to see an html page with the error rendered there. If not, or
-// if something goes wrong during the function, it will recover and just try to serve
-// an appropriate application/json content-type error.
+// ErrorHandler takes the provided gin context and errWithCode
+// and tries to serve a helpful error to the caller.
+//
+// It will do content negotiation to figure out if the caller prefers
+// to see an html page with the error rendered there. If not, or if
+// something goes wrong during the function, it will recover and just
+// try to serve an appropriate application/json content-type error.
// To override the default response type, specify `offers`.
+//
+// If the requester already hung up on the request, ErrorHandler
+// will overwrite the given errWithCode with a 499 error to indicate
+// that the failure wasn't due to something we did, and will avoid
+// trying to write extensive bytes to the caller by just aborting.
+//
+// See: https://en.wikipedia.org/wiki/List_of_HTTP_status_codes#nginx.
func ErrorHandler(c *gin.Context, errWithCode gtserror.WithCode, instanceGet func(ctx context.Context) (*apimodel.InstanceV1, gtserror.WithCode), offers ...MIME) {
- // set the error on the gin context so that it can be logged
- // in the gin logger middleware (internal/router/logger.go)
+ if c.Request.Context().Err() != nil {
+ // Context error means requester probably left already.
+ // Wrap original error with a less alarming one. Then
+ // we can return early, because it doesn't matter what
+ // we send to the client at this point; they're gone.
+ errWithCode = gtserror.NewErrorClientClosedRequest(errWithCode.Unwrap())
+ c.AbortWithStatus(errWithCode.Code())
+ return
+ }
+
+ // Set the error on the gin context so that it can be logged
+ // in the gin logger middleware (internal/middleware/logger.go).
c.Error(errWithCode) //nolint:errcheck
- // discover if we're allowed to serve a nice html error page,
+ // Discover if we're allowed to serve a nice html error page,
// or if we should just use a json. Normally we would want to
// check for a returned error, but if an error occurs here we
// can just fall back to default behavior (serve json error).
accept, _ := NegotiateAccept(c, JSONOrHTMLAcceptHeaders...)
if errWithCode.Code() == http.StatusNotFound {
- // use our special not found handler with useful status text
+ // Use our special not found handler with useful status text.
NotFoundHandler(c, instanceGet, accept)
} else {
genericErrorHandler(c, instanceGet, accept, errWithCode)
diff --git a/internal/gtserror/withcode.go b/internal/gtserror/withcode.go
@@ -23,13 +23,23 @@ import (
"strings"
)
+// Custom http response codes + text that
+// aren't included in the net/http package.
+const (
+ StatusClientClosedRequest = 499
+ StatusTextClientClosedRequest = "Client Closed Request"
+)
+
// WithCode wraps an internal error with an http code, and a 'safe' version of
// the error that can be served to clients without revealing internal business logic.
//
// A typical use of this error would be to first log the Original error, then return
// the Safe error and the StatusCode to an API caller.
type WithCode interface {
- // Error returns the original internal error for debugging within the GoToSocial logs.
+ // Unwrap returns the original error.
+ // This should *NEVER* be returned to a client as it may contain sensitive information.
+ Unwrap() error
+ // Error serializes the original internal error for debugging within the GoToSocial logs.
// This should *NEVER* be returned to a client as it may contain sensitive information.
Error() string
// Safe returns the API-safe version of the error for serialization towards a client.
@@ -45,6 +55,10 @@ type withCode struct {
code int
}
+func (e withCode) Unwrap() error {
+ return e.original
+}
+
func (e withCode) Error() string {
return e.original.Error()
}
@@ -173,3 +187,14 @@ func NewErrorGone(original error, helpText ...string) WithCode {
code: http.StatusGone,
}
}
+
+// NewErrorClientClosedRequest returns an ErrorWithCode 499 with the given original error.
+// This error type should only be used when an http caller has already hung up their request.
+// See: https://en.wikipedia.org/wiki/List_of_HTTP_status_codes#nginx
+func NewErrorClientClosedRequest(original error) WithCode {
+ return withCode{
+ original: original,
+ safe: errors.New(StatusTextClientClosedRequest),
+ code: StatusClientClosedRequest,
+ }
+}
diff --git a/internal/middleware/logger.go b/internal/middleware/logger.go
@@ -27,6 +27,7 @@ import (
"codeberg.org/gruf/go-kv"
"codeberg.org/gruf/go-logger/v2/level"
"github.com/gin-gonic/gin"
+ "github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/log"
)
@@ -91,11 +92,23 @@ func Logger(logClientIP bool) gin.HandlerFunc {
l = l.WithField("error", c.Errors)
}
+ // Get appropriate text for this code.
+ statusText := http.StatusText(code)
+ if statusText == "" {
+ // Look for custom codes.
+ switch code {
+ case gtserror.StatusClientClosedRequest:
+ statusText = gtserror.StatusTextClientClosedRequest
+ default:
+ statusText = "Unknown Status"
+ }
+ }
+
// Generate a nicer looking bytecount
size := bytesize.Size(c.Writer.Size())
- // Finally, write log entry with status text body size
- l.Logf(lvl, "%s: wrote %s", http.StatusText(code), size)
+ // Finally, write log entry with status text + body size.
+ l.Logf(lvl, "%s: wrote %s", statusText, size)
}()
// Process request
