From 93187d760cf74531e68867160884c84e6de4a403 Mon Sep 17 00:00:00 2001 From: Marius Date: Sat, 27 Nov 2021 21:38:26 +0100 Subject: [PATCH] Rework error type from interface to struct --- cmd/tusd/cli/flags.go | 2 - cmd/tusd/cli/hooks.go | 119 +++++++++++++++++--------------- cmd/tusd/cli/hooks/hooks.go | 44 +++++------- cmd/tusd/cli/hooks/http.go | 97 +++++++++++++------------- pkg/handler/config.go | 5 +- pkg/handler/datastore.go | 1 + pkg/handler/metrics.go | 8 +-- pkg/handler/unrouted_handler.go | 114 +++++++++++++++--------------- pkg/s3store/s3store.go | 2 +- 9 files changed, 197 insertions(+), 195 deletions(-) diff --git a/cmd/tusd/cli/flags.go b/cmd/tusd/cli/flags.go index aceb206..3cf1574 100644 --- a/cmd/tusd/cli/flags.go +++ b/cmd/tusd/cli/flags.go @@ -46,7 +46,6 @@ var Flags struct { GrpcHooksEndpoint string GrpcHooksRetry int GrpcHooksBackoff int - HooksStopUploadCode int EnabledHooks []hooks.HookType ShowVersion bool ExposeMetrics bool @@ -98,7 +97,6 @@ func ParseFlags() { flag.StringVar(&Flags.GrpcHooksEndpoint, "hooks-grpc", "", "An gRPC endpoint to which hook events will be sent to") flag.IntVar(&Flags.GrpcHooksRetry, "hooks-grpc-retry", 3, "Number of times to retry on a server error or network timeout") flag.IntVar(&Flags.GrpcHooksBackoff, "hooks-grpc-backoff", 1, "Number of seconds to wait before retrying each retry") - flag.IntVar(&Flags.HooksStopUploadCode, "hooks-stop-code", 0, "Return code from post-receive hook which causes tusd to stop and delete the current upload. A zero value means that no uploads will be stopped") flag.BoolVar(&Flags.ShowVersion, "version", false, "Print tusd version information") flag.BoolVar(&Flags.ExposeMetrics, "expose-metrics", true, "Expose metrics about tusd usage") flag.StringVar(&Flags.MetricsPath, "metrics-path", "/metrics", "Path under which the metrics endpoint will be accessible") diff --git a/cmd/tusd/cli/hooks.go b/cmd/tusd/cli/hooks.go index c6e26c5..7eef693 100644 --- a/cmd/tusd/cli/hooks.go +++ b/cmd/tusd/cli/hooks.go @@ -1,6 +1,7 @@ package cli import ( + "errors" "fmt" "strconv" "strings" @@ -20,27 +21,12 @@ func hookTypeInSlice(a hooks.HookType, list []hooks.HookType) bool { return false } -func hookCallback(typ hooks.HookType, info handler.HookEvent) error { - if output, err := invokeHookSync(typ, info, true); err != nil { - if hookErr, ok := err.(hooks.HookError); ok { - return hooks.NewHookError( - fmt.Errorf("%s hook failed: %s", typ, err), - hookErr.StatusCode(), - hookErr.Body(), - ) - } - return fmt.Errorf("%s hook failed: %s\n%s", typ, err, string(output)) - } - - return nil +func preCreateCallback(event handler.HookEvent) (handler.HTTPResponse, error) { + return invokeHookSync(hooks.HookPreCreate, event) } -func preCreateCallback(info handler.HookEvent) error { - return hookCallback(hooks.HookPreCreate, info) -} - -func preFinishCallback(info handler.HookEvent) error { - return hookCallback(hooks.HookPreFinish, info) +func preFinishCallback(event handler.HookEvent) (handler.HTTPResponse, error) { + return invokeHookSync(hooks.HookPreFinish, event) } func SetupHookMetrics() { @@ -59,13 +45,14 @@ func SetupHookMetrics() { } func SetupPreHooks(config *handler.Config) error { - if Flags.FileHooksDir != "" { - stdout.Printf("Using '%s' for hooks", Flags.FileHooksDir) + // if Flags.FileHooksDir != "" { + // stdout.Printf("Using '%s' for hooks", Flags.FileHooksDir) - hookHandler = &hooks.FileHook{ - Directory: Flags.FileHooksDir, - } - } else if Flags.HttpHooksEndpoint != "" { + // hookHandler = &hooks.FileHook{ + // Directory: Flags.FileHooksDir, + // } + // } else + if Flags.HttpHooksEndpoint != "" { stdout.Printf("Using '%s' as the endpoint for hooks", Flags.HttpHooksEndpoint) hookHandler = &hooks.HttpHook{ @@ -74,14 +61,14 @@ func SetupPreHooks(config *handler.Config) error { Backoff: Flags.HttpHooksBackoff, ForwardHeaders: strings.Split(Flags.HttpHooksForwardHeaders, ","), } - } else if Flags.GrpcHooksEndpoint != "" { - stdout.Printf("Using '%s' as the endpoint for gRPC hooks", Flags.GrpcHooksEndpoint) + // } else if Flags.GrpcHooksEndpoint != "" { + // stdout.Printf("Using '%s' as the endpoint for gRPC hooks", Flags.GrpcHooksEndpoint) - hookHandler = &hooks.GrpcHook{ - Endpoint: Flags.GrpcHooksEndpoint, - MaxRetries: Flags.GrpcHooksRetry, - Backoff: Flags.GrpcHooksBackoff, - } + // hookHandler = &hooks.GrpcHook{ + // Endpoint: Flags.GrpcHooksEndpoint, + // MaxRetries: Flags.GrpcHooksRetry, + // Backoff: Flags.GrpcHooksBackoff, + // } } else { return nil } @@ -107,35 +94,35 @@ func SetupPostHooks(handler *handler.Handler) { go func() { for { select { - case info := <-handler.CompleteUploads: - invokeHookAsync(hooks.HookPostFinish, info) - case info := <-handler.TerminatedUploads: - invokeHookAsync(hooks.HookPostTerminate, info) - case info := <-handler.UploadProgress: - invokeHookAsync(hooks.HookPostReceive, info) - case info := <-handler.CreatedUploads: - invokeHookAsync(hooks.HookPostCreate, info) + case event := <-handler.CompleteUploads: + invokeHookAsync(hooks.HookPostFinish, event) + case event := <-handler.TerminatedUploads: + invokeHookAsync(hooks.HookPostTerminate, event) + case event := <-handler.UploadProgress: + invokeHookAsync(hooks.HookPostReceive, event) + case event := <-handler.CreatedUploads: + invokeHookAsync(hooks.HookPostCreate, event) } } }() } -func invokeHookAsync(typ hooks.HookType, info handler.HookEvent) { +func invokeHookAsync(typ hooks.HookType, event handler.HookEvent) { go func() { // Error handling is taken care by the function. - _, _ = invokeHookSync(typ, info, false) + _, _ = invokeHookSync(typ, event) }() } -func invokeHookSync(typ hooks.HookType, info handler.HookEvent, captureOutput bool) ([]byte, error) { +func invokeHookSync(typ hooks.HookType, event handler.HookEvent) (httpRes handler.HTTPResponse, err error) { if !hookTypeInSlice(typ, Flags.EnabledHooks) { - return nil, nil + return httpRes, nil } MetricsHookInvocationsTotal.WithLabelValues(string(typ)).Add(1) - id := info.Upload.ID - size := info.Upload.Size + id := event.Upload.ID + size := event.Upload.Size switch typ { case hooks.HookPostFinish: @@ -145,28 +132,52 @@ func invokeHookSync(typ hooks.HookType, info handler.HookEvent, captureOutput bo } if hookHandler == nil { - return nil, nil + return httpRes, nil } - name := string(typ) if Flags.VerboseOutput { - logEv(stdout, "HookInvocationStart", "type", name, "id", id) + logEv(stdout, "HookInvocationStart", "type", string(typ), "id", id) } - output, returnCode, err := hookHandler.InvokeHook(typ, info, captureOutput) + hookRes, err := hookHandler.InvokeHook(hooks.HookRequest{ + Type: typ, + Event: event, + }) if err != nil { + err = fmt.Errorf("%s hook failed: %s", typ, err) logEv(stderr, "HookInvocationError", "type", string(typ), "id", id, "error", err.Error()) MetricsHookErrorsTotal.WithLabelValues(string(typ)).Add(1) } else if Flags.VerboseOutput { logEv(stdout, "HookInvocationFinish", "type", string(typ), "id", id) } - if typ == hooks.HookPostReceive && Flags.HooksStopUploadCode != 0 && Flags.HooksStopUploadCode == returnCode { - logEv(stdout, "HookStopUpload", "id", id) + // IDEA: PreHooks work like this: error return value does not carry HTTP response information + // Instead the additional HTTP response return value - info.Upload.StopUpload() + httpRes = hookRes.HTTPResponse + + if hookRes.Error != "" { + // TODO: Is this actually useful? + return httpRes, errors.New(hookRes.Error) } - return output, err + // If the hook response includes the instruction to reject the upload, reuse the error code + // and message from ErrUploadRejectedByServer, but also include custom HTTP response values + if typ == hooks.HookPreCreate && hookRes.RejectUpload { + return httpRes, handler.Error{ + ErrorCode: handler.ErrUploadRejectedByServer.ErrorCode, + Message: handler.ErrUploadRejectedByServer.Message, + HTTPResponse: httpRes, + } + } + + if typ == hooks.HookPostReceive && hookRes.StopUpload { + logEv(stdout, "HookStopUpload", "id", id) + + // TODO: Control response for PATCH request + event.Upload.StopUpload() + } + + return httpRes, err } diff --git a/cmd/tusd/cli/hooks/hooks.go b/cmd/tusd/cli/hooks/hooks.go index b72e3c3..3f13fee 100644 --- a/cmd/tusd/cli/hooks/hooks.go +++ b/cmd/tusd/cli/hooks/hooks.go @@ -6,7 +6,23 @@ import ( type HookHandler interface { Setup() error - InvokeHook(typ HookType, info handler.HookEvent, captureOutput bool) ([]byte, int, error) + InvokeHook(req HookRequest) (res HookResponse, err error) +} + +type HookRequest struct { + Type HookType + Event handler.HookEvent +} + +type HookResponse struct { + // Error indicates whether a fault occurred while processing the hook request. + // If Error is an empty string, no fault is assumed. + Error string + + HTTPResponse handler.HTTPResponse + + RejectUpload bool + StopUpload bool } type HookType string @@ -21,29 +37,3 @@ const ( ) var AvailableHooks []HookType = []HookType{HookPreCreate, HookPostCreate, HookPostReceive, HookPostTerminate, HookPostFinish, HookPreFinish} - -type hookDataStore struct { - handler.DataStore -} - -type HookError struct { - error - statusCode int - body []byte -} - -func NewHookError(err error, statusCode int, body []byte) HookError { - return HookError{err, statusCode, body} -} - -func (herr HookError) StatusCode() int { - return herr.statusCode -} - -func (herr HookError) Body() []byte { - return herr.body -} - -func (herr HookError) Error() string { - return herr.error.Error() -} diff --git a/cmd/tusd/cli/hooks/http.go b/cmd/tusd/cli/hooks/http.go index 4cc3b87..af781de 100644 --- a/cmd/tusd/cli/hooks/http.go +++ b/cmd/tusd/cli/hooks/http.go @@ -8,8 +8,6 @@ import ( "net/http" "time" - "github.com/tus/tusd/pkg/handler" - "github.com/sethgrid/pester" ) @@ -18,35 +16,11 @@ type HttpHook struct { MaxRetries int Backoff int ForwardHeaders []string + + client *pester.Client } -func (_ HttpHook) Setup() error { - return nil -} - -func (h HttpHook) InvokeHook(typ HookType, info handler.HookEvent, captureOutput bool) ([]byte, int, error) { - jsonInfo, err := json.Marshal(info) - if err != nil { - return nil, 0, err - } - - req, err := http.NewRequest("POST", h.Endpoint, bytes.NewBuffer(jsonInfo)) - if err != nil { - return nil, 0, err - } - - for _, k := range h.ForwardHeaders { - // Lookup the Canonicalised version of the specified header - if vals, ok := info.HTTPRequest.Header[http.CanonicalHeaderKey(k)]; ok { - // but set the case specified by the user - req.Header[k] = vals - } - } - - req.Header.Set("Hook-Name", string(typ)) - req.Header.Set("Content-Type", "application/json") - - // TODO: Can we initialize this in Setup()? +func (h *HttpHook) Setup() error { // Use linear backoff strategy with the user defined values. client := pester.New() client.KeepLog = true @@ -55,24 +29,51 @@ func (h HttpHook) InvokeHook(typ HookType, info handler.HookEvent, captureOutput return time.Duration(h.Backoff) * time.Second } - resp, err := client.Do(req) - if err != nil { - return nil, 0, err - } - defer resp.Body.Close() + h.client = client - body, err := ioutil.ReadAll(resp.Body) - if err != nil { - return nil, 0, err - } - - if resp.StatusCode >= http.StatusBadRequest { - return body, resp.StatusCode, NewHookError(fmt.Errorf("endpoint returned: %s", resp.Status), resp.StatusCode, body) - } - - if captureOutput { - return body, resp.StatusCode, err - } - - return nil, resp.StatusCode, err + return nil +} + +func (h HttpHook) InvokeHook(hookReq HookRequest) (hookRes HookResponse, err error) { + jsonInfo, err := json.Marshal(hookReq) + if err != nil { + return hookRes, err + } + + httpReq, err := http.NewRequest("POST", h.Endpoint, bytes.NewBuffer(jsonInfo)) + if err != nil { + return hookRes, err + } + + for _, k := range h.ForwardHeaders { + // Lookup the Canonicalised version of the specified header + if vals, ok := hookReq.Event.HTTPRequest.Header[http.CanonicalHeaderKey(k)]; ok { + // but set the case specified by the user + httpReq.Header[k] = vals + } + } + + httpReq.Header.Set("Content-Type", "application/json") + + httpRes, err := h.client.Do(httpReq) + if err != nil { + return hookRes, err + } + defer httpRes.Body.Close() + + httpBody, err := ioutil.ReadAll(httpRes.Body) + if err != nil { + return hookRes, err + } + + // Report an error, if the response has a non-2XX status code + if httpRes.StatusCode < http.StatusOK || httpRes.StatusCode >= http.StatusMultipleChoices { + return hookRes, fmt.Errorf("unexpected response code from hook endpoint (%d): %s", httpRes.StatusCode, string(httpBody)) + } + + if err = json.Unmarshal(httpBody, &hookRes); err != nil { + return hookRes, fmt.Errorf("failed to parse hook response: %w", err) + } + + return hookRes, nil } diff --git a/pkg/handler/config.go b/pkg/handler/config.go index ae9676b..612dd4d 100644 --- a/pkg/handler/config.go +++ b/pkg/handler/config.go @@ -40,15 +40,16 @@ type Config struct { // potentially set by proxies when generating an absolute URL in the // response to POST requests. RespectForwardedHeaders bool + // TODO: Update comments // PreUploadCreateCallback will be invoked before a new upload is created, if the // property is supplied. If the callback returns nil, the upload will be created. // Otherwise the HTTP request will be aborted. This can be used to implement // validation of upload metadata etc. - PreUploadCreateCallback func(hook HookEvent) error + PreUploadCreateCallback func(hook HookEvent) (HTTPResponse, error) // PreFinishResponseCallback will be invoked after an upload is completed but before // a response is returned to the client. Error responses from the callback will be passed // back to the client. This can be used to implement post-processing validation. - PreFinishResponseCallback func(hook HookEvent) error + PreFinishResponseCallback func(hook HookEvent) (HTTPResponse, error) } func (config *Config) validate() error { diff --git a/pkg/handler/datastore.go b/pkg/handler/datastore.go index 1eb4240..41b0371 100644 --- a/pkg/handler/datastore.go +++ b/pkg/handler/datastore.go @@ -41,6 +41,7 @@ type FileInfo struct { // more data. Furthermore, a response is sent to notify the client of the // interrupting and the upload is terminated (if supported by the data store), // so the upload cannot be resumed anymore. +// TODO: Allow passing in a HTTP Response func (f FileInfo) StopUpload() { if f.stopUpload != nil { f.stopUpload() diff --git a/pkg/handler/metrics.go b/pkg/handler/metrics.go index 699086e..57a536e 100644 --- a/pkg/handler/metrics.go +++ b/pkg/handler/metrics.go @@ -31,7 +31,7 @@ func (m Metrics) incRequestsTotal(method string) { // TODO: Rework to only store error code // incErrorsTotal increases the counter for this error atomically by one. -func (m Metrics) incErrorsTotal(err HTTPError) { +func (m Metrics) incErrorsTotal(err Error) { ptr := m.ErrorsTotal.retrievePointerFor(err) atomic.AddUint64(ptr, 1) } @@ -95,10 +95,10 @@ func newErrorsTotalMap() *ErrorsTotalMap { // retrievePointerFor returns (after creating it if necessary) the pointer to // the counter for the error. -func (e *ErrorsTotalMap) retrievePointerFor(err HTTPError) *uint64 { +func (e *ErrorsTotalMap) retrievePointerFor(err Error) *uint64 { serr := ErrorsTotalMapEntry{ - ErrorCode: err.ErrorCode(), - StatusCode: err.StatusCode(), + ErrorCode: err.ErrorCode, + StatusCode: err.HTTPResponse.StatusCode, } e.lock.RLock() diff --git a/pkg/handler/unrouted_handler.go b/pkg/handler/unrouted_handler.go index 7cb37a0..4fd23b8 100644 --- a/pkg/handler/unrouted_handler.go +++ b/pkg/handler/unrouted_handler.go @@ -23,69 +23,60 @@ var ( reMimeType = regexp.MustCompile(`^[a-z]+\/[a-z0-9\-\+\.]+$`) ) -// HTTPError represents an error with an additional status code attached -// which may be used when this error is sent in a HTTP response. -// See the net/http package for standardized status codes. -type HTTPError interface { - error - ErrorCode() string - StatusCode() int - Body() []byte +// TODO: Move in own file +// ErrorWithResponse represents an error with an additional HTTP response +// attached, which can hold a status code, body and headers. +type Error struct { + ErrorCode string + Message string + HTTPResponse HTTPResponse } -type httpError struct { - errorCode string - message string - statusCode int +func (e Error) Error() string { + return e.ErrorCode + ": " + e.Message } -func (err httpError) Error() string { - return err.errorCode + ": " + err.message -} - -func (err httpError) StatusCode() int { - return err.statusCode -} - -func (err httpError) ErrorCode() string { - return err.errorCode -} - -func (err httpError) Body() []byte { - return []byte(err.Error()) -} - -// NewHTTPError adds the given status code to the provided error and returns +// TODO: Rename comment +// NewError adds the given status code to the provided error and returns // the new error instance. The status code may be used in corresponding HTTP // responses. See the net/http package for standardized status codes. -func NewHTTPError(errCode string, message string, statusCode int) HTTPError { - return httpError{errCode, message, statusCode} +func NewError(errCode string, message string, statusCode int) Error { + return Error{ + ErrorCode: errCode, + Message: message, + HTTPResponse: HTTPResponse{ + StatusCode: statusCode, + Body: []byte(errCode + ": " + message), + }, + } } var ( - ErrUnsupportedVersion = NewHTTPError("ERR_UNSUPPORTED_VERSION", "missing, invalid or unsupported Tus-Resumable header", http.StatusPreconditionFailed) - ErrMaxSizeExceeded = NewHTTPError("ERR_MAX_SIZE_EXCEEDED", "maximum size exceeded", http.StatusRequestEntityTooLarge) - ErrInvalidContentType = NewHTTPError("ERR_INVALID_CONTENT_TYPE", "missing or invalid Content-Type header", http.StatusBadRequest) - ErrInvalidUploadLength = NewHTTPError("ERR_INVALID_UPLOAD_LENGTH", "missing or invalid Upload-Length header", http.StatusBadRequest) - ErrInvalidOffset = NewHTTPError("ERR_INVALID_OFFSET", "missing or invalid Upload-Offset header", http.StatusBadRequest) - ErrNotFound = NewHTTPError("ERR_UPLOAD_NOT_FOUND", "upload not found", http.StatusNotFound) - ErrFileLocked = NewHTTPError("ERR_UPLOAD_LOCKED", "file currently locked", http.StatusLocked) - ErrMismatchOffset = NewHTTPError("ERR_MISMATCHED_OFFSET", "mismatched offset", http.StatusConflict) - ErrSizeExceeded = NewHTTPError("ERR_UPLOAD_SIZE_EXCEEDED", "upload's size exceeded", http.StatusRequestEntityTooLarge) - ErrNotImplemented = NewHTTPError("ERR_NOT_IMPLEMENTED", "feature not implemented", http.StatusNotImplemented) - ErrUploadNotFinished = NewHTTPError("ERR_UPLOAD_NOT_FINISHED", "one of the partial uploads is not finished", http.StatusBadRequest) - ErrInvalidConcat = NewHTTPError("ERR_INVALID_CONCAT", "invalid Upload-Concat header", http.StatusBadRequest) - ErrModifyFinal = NewHTTPError("ERR_MODIFY_FINAL", "modifying a final upload is not allowed", http.StatusForbidden) - ErrUploadLengthAndUploadDeferLength = NewHTTPError("ERR_AMBIGUOUS_UPLOAD_LENGTH", "provided both Upload-Length and Upload-Defer-Length", http.StatusBadRequest) - ErrInvalidUploadDeferLength = NewHTTPError("ERR_INVALID_UPLOAD_LENGTH_DEFER", "invalid Upload-Defer-Length header", http.StatusBadRequest) - ErrUploadStoppedByServer = NewHTTPError("ERR_UPLOAD_STOPPED", "upload has been stopped by server", http.StatusBadRequest) + ErrUnsupportedVersion = NewError("ERR_UNSUPPORTED_VERSION", "missing, invalid or unsupported Tus-Resumable header", http.StatusPreconditionFailed) + ErrMaxSizeExceeded = NewError("ERR_MAX_SIZE_EXCEEDED", "maximum size exceeded", http.StatusRequestEntityTooLarge) + ErrInvalidContentType = NewError("ERR_INVALID_CONTENT_TYPE", "missing or invalid Content-Type header", http.StatusBadRequest) + ErrInvalidUploadLength = NewError("ERR_INVALID_UPLOAD_LENGTH", "missing or invalid Upload-Length header", http.StatusBadRequest) + ErrInvalidOffset = NewError("ERR_INVALID_OFFSET", "missing or invalid Upload-Offset header", http.StatusBadRequest) + ErrNotFound = NewError("ERR_UPLOAD_NOT_FOUND", "upload not found", http.StatusNotFound) + ErrFileLocked = NewError("ERR_UPLOAD_LOCKED", "file currently locked", http.StatusLocked) + ErrMismatchOffset = NewError("ERR_MISMATCHED_OFFSET", "mismatched offset", http.StatusConflict) + ErrSizeExceeded = NewError("ERR_UPLOAD_SIZE_EXCEEDED", "upload's size exceeded", http.StatusRequestEntityTooLarge) + ErrNotImplemented = NewError("ERR_NOT_IMPLEMENTED", "feature not implemented", http.StatusNotImplemented) + ErrUploadNotFinished = NewError("ERR_UPLOAD_NOT_FINISHED", "one of the partial uploads is not finished", http.StatusBadRequest) + ErrInvalidConcat = NewError("ERR_INVALID_CONCAT", "invalid Upload-Concat header", http.StatusBadRequest) + ErrModifyFinal = NewError("ERR_MODIFY_FINAL", "modifying a final upload is not allowed", http.StatusForbidden) + ErrUploadLengthAndUploadDeferLength = NewError("ERR_AMBIGUOUS_UPLOAD_LENGTH", "provided both Upload-Length and Upload-Defer-Length", http.StatusBadRequest) + ErrInvalidUploadDeferLength = NewError("ERR_INVALID_UPLOAD_LENGTH_DEFER", "invalid Upload-Defer-Length header", http.StatusBadRequest) + ErrUploadStoppedByServer = NewError("ERR_UPLOAD_STOPPED", "upload has been stopped by server", http.StatusBadRequest) + ErrUploadRejectedByServer = NewError("ERR_UPLOAD_REJECTED", "upload creation has been rejected by server", http.StatusBadRequest) // TODO: These two responses are 500 for backwards compatability. We should discuss // whether it is better to more them to 4XX status codes. - ErrReadTimeout = NewHTTPError("ERR_READ_TIMEOUT", "timeout while reading request body", http.StatusInternalServerError) - ErrConnectionReset = NewHTTPError("ERR_CONNECTION_RESET", "TCP connection reset by peer", http.StatusInternalServerError) + ErrReadTimeout = NewError("ERR_READ_TIMEOUT", "timeout while reading request body", http.StatusInternalServerError) + ErrConnectionReset = NewError("ERR_CONNECTION_RESET", "TCP connection reset by peer", http.StatusInternalServerError) ) +// TODO: Move HTTP structs into own file // HTTPRequest contains basic details of an incoming HTTP request. type HTTPRequest struct { // Method is the HTTP method, e.g. POST or PATCH @@ -98,6 +89,15 @@ type HTTPRequest struct { Header http.Header } +type HTTPResponse struct { + // HTTPStatus, HTTPHeaders and HTTPBody control these details of the corresponding + // HTTP response. + // TODO: Currently only works for error responses + StatusCode int + Headers http.Header + Body []byte +} + // HookEvent represents an event from tusd which can be handled by the application. type HookEvent struct { // Upload contains information about the upload that caused this hook @@ -354,7 +354,7 @@ func (handler *UnroutedHandler) PostFile(w http.ResponseWriter, r *http.Request) } if handler.config.PreUploadCreateCallback != nil { - if err := handler.config.PreUploadCreateCallback(newHookEvent(info, r)); err != nil { + if _, err := handler.config.PreUploadCreateCallback(newHookEvent(info, r)); err != nil { handler.sendError(w, r, err) return } @@ -710,7 +710,7 @@ func (handler *UnroutedHandler) finishUploadIfComplete(ctx context.Context, uplo handler.Metrics.incUploadsFinished() if handler.config.PreFinishResponseCallback != nil { - if err := handler.config.PreFinishResponseCallback(newHookEvent(info, r)); err != nil { + if _, err := handler.config.PreFinishResponseCallback(newHookEvent(info, r)); err != nil { return err } } @@ -950,13 +950,13 @@ func (handler *UnroutedHandler) sendError(w http.ResponseWriter, r *http.Request // err = nil //} - statusErr, ok := err.(HTTPError) + detailedErr, ok := err.(Error) if !ok { handler.log("InternalServerError", "message", err.Error(), "method", r.Method, "path", r.URL.Path, "requestId", getRequestId(r)) - statusErr = NewHTTPError("ERR_INTERNAL_SERVER_ERROR", err.Error(), http.StatusInternalServerError) + detailedErr = NewError("ERR_INTERNAL_SERVER_ERROR", err.Error(), http.StatusInternalServerError) } - reason := append(statusErr.Body(), '\n') + reason := append(detailedErr.HTTPResponse.Body, '\n') if r.Method == "HEAD" { reason = nil } @@ -964,12 +964,12 @@ func (handler *UnroutedHandler) sendError(w http.ResponseWriter, r *http.Request // TODO: Allow JSON response w.Header().Set("Content-Type", "text/plain; charset=utf-8") w.Header().Set("Content-Length", strconv.Itoa(len(reason))) - w.WriteHeader(statusErr.StatusCode()) + w.WriteHeader(detailedErr.HTTPResponse.StatusCode) w.Write(reason) - handler.log("ResponseOutgoing", "status", strconv.Itoa(statusErr.StatusCode()), "method", r.Method, "path", r.URL.Path, "error", statusErr.ErrorCode(), "requestId", getRequestId(r)) + handler.log("ResponseOutgoing", "status", strconv.Itoa(detailedErr.HTTPResponse.StatusCode), "method", r.Method, "path", r.URL.Path, "error", detailedErr.ErrorCode, "requestId", getRequestId(r)) - handler.Metrics.incErrorsTotal(statusErr) + handler.Metrics.incErrorsTotal(detailedErr) } // sendResp writes the header to w with the specified status code. diff --git a/pkg/s3store/s3store.go b/pkg/s3store/s3store.go index 91f191d..93d76f7 100644 --- a/pkg/s3store/s3store.go +++ b/pkg/s3store/s3store.go @@ -719,7 +719,7 @@ func (upload s3Upload) GetReader(ctx context.Context) (io.Reader, error) { }) if err == nil { // The multipart upload still exists, which means we cannot download it yet - return nil, handler.NewHTTPError("ERR_INCOMPLETE_UPLOAD", "cannot stream non-finished upload", http.StatusBadRequest) + return nil, handler.NewError("ERR_INCOMPLETE_UPLOAD", "cannot stream non-finished upload", http.StatusBadRequest) } if isAwsError(err, "NoSuchUpload") {