Merge pull request #41 from danielgtaylor/error-status

feat: return more specific error codes

README.md

@@ -276,11 +276,32 @@ app.Resource("/exhaustive").Get("exhaustive", "Exhastive errors example",
 })
 ```
 
+While every attempt is made to return exhaustive errors within Huma, each individual response can only contain a single HTTP status code. The following chart describes which codes get returned and when:
+
+```mermaid
+flowchart TD
+	Request[Request has errors?] -->|yes| Panic
+	Request -->|no| Continue[Continue to handler]
+	Panic[Panic?] -->|yes| 500
+	Panic -->|no| RequestBody[Request body too large?]
+	RequestBody -->|yes| 413
+	RequestBody -->|no| RequestTimeout[Request took too long to read?]
+	RequestTimeout -->|yes| 408
+	RequestTimeout -->|no| ParseFailure[Cannot parse input?]
+	ParseFailure -->|yes| 400
+	ParseFailure -->|no| ValidationFailure[Validation failed?]
+	ValidationFailure -->|yes| 422
+	ValidationFailure -->|no| 400
+```
+
+This means it is possible to, for example, get an HTTP `408 Request Timeout` response that _also_ contains an error detail with a validation error for one of the input headers. Since request timeout has higher priority, that will be the response status code that is returned.
+
 ## WriteContent
 
-Write contents allows you to write content in the provided ReadSeeker in the response.  It will handle Range, If-Match, If-Unmodified-Since, If-None-Match, If-Modified-Since, and if-Range requests for caching and large object partial content responses.
+Write contents allows you to write content in the provided ReadSeeker in the response. It will handle Range, If-Match, If-Unmodified-Since, If-None-Match, If-Modified-Since, and if-Range requests for caching and large object partial content responses.
 
 example of a partial content response sending a large video file:
+
 ```go
 package main
 
@@ -328,7 +349,7 @@ func main() {
 }
 ```
 
-Note that `WriteContent` does not automatically set the mime type.  You should set the `Content-Type` response header directly.  Also in order for `WriteContent` to respect the `Modified` headers you must call `SetContentLastModified`.  This is optional and if not set `WriteContent` will simply not respect the `Modified` request headers.
+Note that `WriteContent` does not automatically set the mime type. You should set the `Content-Type` response header directly. Also in order for `WriteContent` to respect the `Modified` headers you must call `SetContentLastModified`. This is optional and if not set `WriteContent` will simply not respect the `Modified` request headers.
 
 ## Request Inputs
 

context.go

@@ -78,6 +78,7 @@ type hcontext struct {
 	http.ResponseWriter
 	r                     *http.Request
 	errors                []error
+	errorCode             int
 	op                    *Operation
 	closed                bool
 	docsPrefix            string

operation.go

@@ -220,9 +220,20 @@ func (o *Operation) Run(handler interface{}) {
 			}
 		}
 
+		possible := []int{http.StatusBadRequest}
+
+		if _, ok := input.FieldByName("Body"); ok || len(o.params) > 0 {
+			// Invalid parameter values or body values can cause a 422.
+			possible = append(possible, http.StatusUnprocessableEntity)
+		}
+
 		// Get body if present.
 		if body, ok := input.FieldByName("Body"); ok {
 			o.requestModel = body.Type
+			possible = append(possible,
+				http.StatusRequestEntityTooLarge,
+				http.StatusRequestTimeout,
+			)
 
 			if o.requestSchema == nil {
 				o.requestSchema, err = schema.GenerateWithMode(body.Type, schema.ModeWrite, nil)
@@ -232,18 +243,17 @@ func (o *Operation) Run(handler interface{}) {
 			}
 		}
 
-		// It's possible for the inputs to generate a 400, so add it if it wasn't
-		// explicitly defined.
-		found400 := false
+		// It's possible for the inputs to generate a few different errors, so
+		// generate them if not already present.
+		found := map[int]bool{}
 		for _, r := range o.responses {
-			if r.status == http.StatusBadRequest {
-				found400 = true
-				break
-			}
+			found[r.status] = true
 		}
 
-		if !found400 {
-			o.responses = append(o.responses, NewResponse(http.StatusBadRequest, http.StatusText(http.StatusBadRequest)).ContentType("application/problem+json").Model(&ErrorModel{}))
+		for _, s := range possible {
+			if !found[s] {
+				o.responses = append(o.responses, NewResponse(s, http.StatusText(s)).ContentType("application/problem+json").Model(&ErrorModel{}))
+			}
 		}
 	}
 
@@ -266,6 +276,7 @@ func (o *Operation) Run(handler interface{}) {
 			docsPrefix:            o.resource.router.docsPrefix,
 			urlPrefix:             o.resource.router.urlPrefix,
 			disableSchemaProperty: o.resource.router.disableSchemaProperty,
+			errorCode:             http.StatusBadRequest,
 		}
 
 		// If there is no input struct (just a context), then the call is simple.
@@ -296,9 +307,14 @@ func (o *Operation) Run(handler interface{}) {
 		}
 
 		setFields(ctx, ctx.r, input, inputType)
+		if !ctx.HasError() {
+			// No errors yet, so any errors that come after should be treated as a
+			// semantic rather than structural error.
+			ctx.errorCode = http.StatusUnprocessableEntity
+		}
 		resolveFields(ctx, "", input)
 		if ctx.HasError() {
-			ctx.WriteError(http.StatusBadRequest, "Error while parsing input parameters")
+			ctx.WriteError(ctx.errorCode, "Error while processing input parameters")
 			return
 		}
 

resolver.go

@@ -70,6 +70,10 @@ func validAgainstSchema(ctx *hcontext, label string, schema *schema.Schema, data
 		for _, desc := range result.Errors() {
 			// Note: some descriptions start with the context location so we trim
 			// those off to prevent duplicating data. (e.g. see the enum error)
+			if ctx.errorCode <= 400 {
+				// Set if a more specific code hasn't been set yet.
+				ctx.errorCode = http.StatusUnprocessableEntity
+			}
 			ctx.AddError(&ErrorDetail{
 				Message:  strings.TrimPrefix(desc.Description(), desc.Context().String()+" "),
 				Location: strings.TrimSuffix(label+strings.TrimPrefix(desc.Field(), "(root)"), "."),
@@ -201,6 +205,7 @@ func setFields(ctx *hcontext, req *http.Request, input reflect.Value, t reflect.
 			if length := req.Header.Get("Content-Length"); length != "" {
 				if l, err := strconv.ParseInt(length, 10, 64); err == nil {
 					if l > ctx.op.maxBodyBytes {
+						ctx.errorCode = http.StatusRequestEntityTooLarge
 						ctx.AddError(&ErrorDetail{
 							Message:  fmt.Sprintf("Request body too large, limit = %d bytes", ctx.op.maxBodyBytes),
 							Location: locationBody,
@@ -215,11 +220,13 @@ func setFields(ctx *hcontext, req *http.Request, input reflect.Value, t reflect.
 			data, err := ioutil.ReadAll(req.Body)
 			if err != nil {
 				if strings.Contains(err.Error(), "request body too large") {
+					ctx.errorCode = http.StatusRequestEntityTooLarge
 					ctx.AddError(&ErrorDetail{
 						Message:  fmt.Sprintf("Request body too large, limit = %d bytes", ctx.op.maxBodyBytes),
 						Location: locationBody,
 					})
 				} else if e, ok := err.(net.Error); ok && e.Timeout() {
+					ctx.errorCode = http.StatusRequestTimeout
 					ctx.AddError(&ErrorDetail{
 						Message:  fmt.Sprintf("Request body took too long to read: timed out after %v", ctx.op.bodyReadTimeout),
 						Location: locationBody,
@@ -238,7 +245,11 @@ func setFields(ctx *hcontext, req *http.Request, input reflect.Value, t reflect.
 
 			err = json.Unmarshal(data, inField.Addr().Interface())
 			if err != nil {
-				panic(err)
+				ctx.AddError(&ErrorDetail{
+					Message:  "Cannot unmarshal JSON request body",
+					Location: locationBody,
+					Value:    string(data),
+				})
 			}
 			continue
 		}

resolver_test.go

@@ -34,7 +34,7 @@ func TestExhaustiveErrors(t *testing.T) {
 	r.Host = "example.com"
 	app.ServeHTTP(w, r)
 
-	assert.JSONEq(t, `{"$schema": "https://example.com/schemas/ErrorModel.json", "title":"Bad Request","status":400,"detail":"Error while parsing input parameters","errors":[{"message":"cannot parse boolean","location":"query.bool","value":"bad"},{"message":"cannot parse integer","location":"query.int","value":"bad"},{"message":"cannot parse float","location":"query.float32","value":"bad"},{"message":"cannot parse float","location":"query.float64","value":"bad"},{"message":"cannot parse integer","location":"query[2].tags","value":"bad"},{"message":"unable to validate against schema: invalid character 'b' looking for beginning of value","location":"query.tags","value":"[1,2,bad]"},{"message":"cannot parse time","location":"query.time","value":"bad"},{"message":"Must be greater than or equal to 5","location":"body.test","value":1}]}`, w.Body.String())
+	assert.JSONEq(t, `{"$schema": "https://example.com/schemas/ErrorModel.json", "title":"Unprocessable Entity","status":422,"detail":"Error while processing input parameters","errors":[{"message":"cannot parse boolean","location":"query.bool","value":"bad"},{"message":"cannot parse integer","location":"query.int","value":"bad"},{"message":"cannot parse float","location":"query.float32","value":"bad"},{"message":"cannot parse float","location":"query.float64","value":"bad"},{"message":"cannot parse integer","location":"query[2].tags","value":"bad"},{"message":"unable to validate against schema: invalid character 'b' looking for beginning of value","location":"query.tags","value":"[1,2,bad]"},{"message":"cannot parse time","location":"query.time","value":"bad"},{"message":"Must be greater than or equal to 5","location":"body.test","value":1}]}`, w.Body.String())
 }
 
 type Dep1 struct {
@@ -110,9 +110,9 @@ func TestNestedResolverError(t *testing.T) {
 
 	assert.JSONEq(t, `{
 		"$schema": "https://example.com/schemas/ErrorModel.json",
-		"status": 400,
-		"title": "Bad Request",
-		"detail": "Error while parsing input parameters",
+		"status": 422,
+		"title": "Unprocessable Entity",
+		"detail": "Error while processing input parameters",
 		"errors": [
 			{
 				"message": "Only one of ['one', 'two'] is allowed.",
@@ -122,3 +122,22 @@ func TestNestedResolverError(t *testing.T) {
 		]
 	}`, w.Body.String())
 }
+
+func TestInvalidJSON(t *testing.T) {
+	app := newTestRouter()
+
+	app.Resource("/").Post("test", "Test",
+		NewResponse(http.StatusNoContent, "desc"),
+	).Run(func(ctx Context, input struct {
+		Body string
+	}) {
+		ctx.WriteHeader(http.StatusNoContent)
+	})
+
+	// Test happy case just sending ONE of the two possible fields in each struct.
+	w := httptest.NewRecorder()
+	r, _ := http.NewRequest(http.MethodPost, "/", strings.NewReader(`{.2asdf2`))
+	app.ServeHTTP(w, r)
+
+	assert.Equal(t, http.StatusBadRequest, w.Result().StatusCode)
+}

router_test.go

@@ -198,15 +198,15 @@ func TestTooBigBody(t *testing.T) {
 	w := httptest.NewRecorder()
 	req, _ := http.NewRequest(http.MethodPut, "/test", strings.NewReader(`{"id": "foo"}`))
 	app.ServeHTTP(w, req)
-	assert.Equal(t, http.StatusBadRequest, w.Code)
+	assert.Equal(t, http.StatusRequestEntityTooLarge, w.Code)
 	assert.Contains(t, w.Body.String(), "Request body too large")
 
 	// With content length
 	w = httptest.NewRecorder()
 	req, _ = http.NewRequest(http.MethodPut, "/test", strings.NewReader(`{"id": "foo"}`))
 	req.Header.Set("Content-Length", "13")
 	app.ServeHTTP(w, req)
-	assert.Equal(t, http.StatusBadRequest, w.Code)
+	assert.Equal(t, http.StatusRequestEntityTooLarge, w.Code)
 	assert.Contains(t, w.Body.String(), "Request body too large")
 }
 
@@ -250,7 +250,7 @@ func TestBodySlow(t *testing.T) {
 	w := httptest.NewRecorder()
 	req, _ := http.NewRequest(http.MethodPut, "/test", &slowReader{})
 	app.ServeHTTP(w, req)
-	assert.Equal(t, http.StatusBadRequest, w.Code)
+	assert.Equal(t, http.StatusRequestTimeout, w.Code)
 	assert.Contains(t, w.Body.String(), "timed out")
 }
 
@@ -414,7 +414,7 @@ func TestCustomRequestSchema(t *testing.T) {
 	req, _ := http.NewRequest(http.MethodPost, "/foo", strings.NewReader("1234"))
 	app.ServeHTTP(w, req)
 
-	assert.Equal(t, http.StatusBadRequest, w.Result().StatusCode)
+	assert.Equal(t, http.StatusUnprocessableEntity, w.Result().StatusCode)
 }
 
 func TestGetOperationName(t *testing.T) {