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

[feature/performance] Wrap incoming HTTP requests in timeout handler #2353

Merged
merged 7 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/api/activitypub.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type ActivityPub struct {
signatureCheckMiddleware gin.HandlerFunc
}

func (a *ActivityPub) Route(r router.Router, m ...gin.HandlerFunc) {
func (a *ActivityPub) Route(r *router.Router, m ...gin.HandlerFunc) {
// create groupings for the 'emoji' and 'users' prefixes
emojiGroup := r.AttachGroup("emoji")
usersGroup := r.AttachGroup("users")
Expand All @@ -54,7 +54,7 @@ func (a *ActivityPub) Route(r router.Router, m ...gin.HandlerFunc) {
}

// Public key endpoint requires different middleware + cache policies from other AP endpoints.
func (a *ActivityPub) RoutePublicKey(r router.Router, m ...gin.HandlerFunc) {
func (a *ActivityPub) RoutePublicKey(r *router.Router, m ...gin.HandlerFunc) {
// Create grouping for the 'users/[username]/main-key' prefix.
publicKeyGroup := r.AttachGroup(publickey.PublicKeyPath)

Expand Down
2 changes: 1 addition & 1 deletion internal/api/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type Auth struct {
}

// Route attaches 'auth' and 'oauth' groups to the given router.
func (a *Auth) Route(r router.Router, m ...gin.HandlerFunc) {
func (a *Auth) Route(r *router.Router, m ...gin.HandlerFunc) {
// create groupings for the 'auth' and 'oauth' prefixes
authGroup := r.AttachGroup("auth")
oauthGroup := r.AttachGroup("oauth")
Expand Down
2 changes: 1 addition & 1 deletion internal/api/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ type Client struct {
user *user.Module // api/v1/user
}

func (c *Client) Route(r router.Router, m ...gin.HandlerFunc) {
func (c *Client) Route(r *router.Router, m ...gin.HandlerFunc) {
// create a new group on the top level client 'api' prefix
apiGroup := r.AttachGroup("api")

Expand Down
2 changes: 1 addition & 1 deletion internal/api/fileserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type Fileserver struct {
fileserver *fileserver.Module
}

func (f *Fileserver) Route(r router.Router, m ...gin.HandlerFunc) {
func (f *Fileserver) Route(r *router.Router, m ...gin.HandlerFunc) {
fileserverGroup := r.AttachGroup("fileserver")

// Attach middlewares appropriate for this group.
Expand Down
2 changes: 1 addition & 1 deletion internal/api/nodeinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type NodeInfo struct {
nodeInfo *nodeinfo.Module
}

func (w *NodeInfo) Route(r router.Router, m ...gin.HandlerFunc) {
func (w *NodeInfo) Route(r *router.Router, m ...gin.HandlerFunc) {
// group nodeinfo endpoints together
nodeInfoGroup := r.AttachGroup("nodeinfo")

Expand Down
46 changes: 34 additions & 12 deletions internal/api/util/errorhandling.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
"github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/log"
"github.com/superseriousbusiness/gotosocial/internal/router"
)

// TODO: add more templated html pages here for different error types
Expand Down Expand Up @@ -90,19 +91,40 @@ func genericErrorHandler(c *gin.Context, instanceGet func(ctx context.Context) (
// 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.
// If the requester already hung up on the request, or the server
// timed out a very slow request, ErrorHandler will overwrite the
// given errWithCode with a 408 or 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) {
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())
// For 499, 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,
) {
if ctxErr := c.Request.Context().Err(); ctxErr != nil {
// Context error means either client has left already,
// or server has timed out a very slow request.
//
// Rewrap the error with something less scary,
// and just abort the request gracelessly.
err := errWithCode.Unwrap()

if ctxErr == router.ErrRequestTimeout {
tsmethurst marked this conversation as resolved.
Show resolved Hide resolved
// We timed out the request.
errWithCode = gtserror.NewErrorRequestTimeout(err)

// Be correct and write "close".
// See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Connection#close
// and: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/408
c.Header("Connection", "close")
} else {
// Client timed out the request.
errWithCode = gtserror.NewErrorClientClosedRequest(err)
}

c.AbortWithStatus(errWithCode.Code())
return
}
Expand Down
2 changes: 1 addition & 1 deletion internal/api/wellknown.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type WellKnown struct {
hostMeta *hostmeta.Module
}

func (w *WellKnown) Route(r router.Router, m ...gin.HandlerFunc) {
func (w *WellKnown) Route(r *router.Router, m ...gin.HandlerFunc) {
// group .well-known endpoints together
wellKnownGroup := r.AttachGroup(".well-known")

Expand Down
4 changes: 2 additions & 2 deletions internal/gotosocial/gotosocial.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ import (
// GoToSocial server instance.
type Server struct {
db db.DB
apiRouter router.Router
apiRouter *router.Router
cleaner *cleaner.Cleaner
}

// NewServer returns a new
// GoToSocial server instance.
func NewServer(
db db.DB,
apiRouter router.Router,
apiRouter *router.Router,
cleaner *cleaner.Cleaner,
) *Server {
return &Server{
Expand Down
11 changes: 11 additions & 0 deletions internal/gtserror/withcode.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,3 +198,14 @@ func NewErrorClientClosedRequest(original error) WithCode {
code: StatusClientClosedRequest,
}
}

// NewErrorRequestTimeout returns an ErrorWithCode 408 with the given original error.
// This error type should only be used when the server has decided to hang up a client
// request after x amount of time, to avoid keeping extremely slow client requests open.
func NewErrorRequestTimeout(original error) WithCode {
return withCode{
original: original,
safe: errors.New(http.StatusText(http.StatusRequestTimeout)),
code: http.StatusRequestTimeout,
}
}
8 changes: 4 additions & 4 deletions internal/router/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,18 @@ package router

import "github.com/gin-gonic/gin"

func (r *router) AttachGlobalMiddleware(handlers ...gin.HandlerFunc) gin.IRoutes {
func (r *Router) AttachGlobalMiddleware(handlers ...gin.HandlerFunc) gin.IRoutes {
return r.engine.Use(handlers...)
}

func (r *router) AttachNoRouteHandler(handler gin.HandlerFunc) {
func (r *Router) AttachNoRouteHandler(handler gin.HandlerFunc) {
r.engine.NoRoute(handler)
}

func (r *router) AttachGroup(relativePath string, handlers ...gin.HandlerFunc) *gin.RouterGroup {
func (r *Router) AttachGroup(relativePath string, handlers ...gin.HandlerFunc) *gin.RouterGroup {
return r.engine.Group(relativePath, handlers...)
}

func (r *router) AttachHandler(method string, path string, handler gin.HandlerFunc) {
func (r *Router) AttachHandler(method string, path string, handler gin.HandlerFunc) {
r.engine.Handle(method, path, handler)
}
Loading