Skip to content

Commit

Permalink
Error chaining and handler selection (#595)
Browse files Browse the repository at this point in the history
* wrap error with errors.Sequence if it is not already an errors.Sequence

* get _all_ kinds of inner error _first_

* now the backend timeout error is wrapped with a sequence error, resulting in a different HTTP status code

* added test cases for error handler selection: because an error occurs in a request sequence, the default error handling has endpoint error for client, whereas the specific error is logged

* changelog entry

* Simplify code

Co-authored-by: Alex Schneider <alex.schneider@avenga.com>
  • Loading branch information
johakoch and Alex Schneider committed Sep 20, 2022
1 parent 315c8d2 commit 51ac083
Show file tree
Hide file tree
Showing 6 changed files with 538 additions and 19 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Unreleased changes are available as `avenga/couper:edge` container.
* **Fixed**
* Aligned the evaluation of [`beta_oauth2`](https://docs.couper.io/configuration/block/oauth2_ac)/[`oidc`](https://docs.couper.io/configuration/block/oidc) `redirect_uri` to `saml` `sp_acs_url` ([#589](https://github.com/avenga/couper/pull/589))
* Proper handling of empty [`beta_oauth2`](https://docs.couper.io/configuration/block/oauth2_ac)/[`oidc`](https://docs.couper.io/configuration/block/oidc) `scope` ([#593](https://github.com/avenga/couper/pull/593))
* Throwing [sequence errors](https://docs.couper.io/configuration/error-handling#endpoint-error-types) and selecting appropriate [error handlers](https://docs.couper.io/configuration/error-handling) ([#595](https://github.com/avenga/couper/pull/595))

---

Expand Down
16 changes: 5 additions & 11 deletions errors/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,26 +58,20 @@ func (e *Error) Kind(name string) *Error {
err := e.clone()
e.isParent = true
err.isParent = false
err.kinds = append(err.kinds, name)
err.kinds = append([]string{name}, err.kinds...)
return err
}

// Kinds returns all configured kinds in reversed order so the
// Kinds returns all configured kinds, the
// most specific one gets evaluated first.
func (e *Error) Kinds() []string {
var reversed []string
for i := len(e.kinds); i > 0; i-- {
reversed = append(reversed, e.kinds[i-1])
}
var kinds []string

if eer, ok := e.inner.(*Error); ok {
k := eer.Kinds()
if len(k) > 0 {
reversed = append(reversed, k[0])
}
kinds = eer.Kinds()[:]
}

return reversed
return append(kinds, e.kinds...)
}

// Context appends the given context block type to the existing ones.
Expand Down
13 changes: 12 additions & 1 deletion handler/producer/sequence.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ func (s Sequence) Produce(req *http.Request, results chan<- *Result) {
}

if lastResult.Err != nil {
if _, ok := lastResult.Err.(*errors.Error); !ok {
// only wrap if lastResult.Err is not already an errors.Sequence
cErr, ok := lastResult.Err.(*errors.Error)
if !ok || !hasSequenceKind(cErr) {
lastResult.Err = errors.Sequence.With(lastResult.Err)
}
results <- lastResult
Expand All @@ -99,6 +101,15 @@ func (s Sequence) Produce(req *http.Request, results chan<- *Result) {
results <- lastResult
}

func hasSequenceKind(cerr *errors.Error) bool {
for _, kind := range cerr.Kinds() {
if kind == "sequence" {
return true
}
}
return false
}

func (s Sequence) Len() int {
var sum int
for _, t := range s {
Expand Down
22 changes: 20 additions & 2 deletions server/http_endpoints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -760,8 +760,8 @@ func TestEndpointSequenceBackendTimeout(t *testing.T) {
helper.Must(err)
}

if res.StatusCode != http.StatusGatewayTimeout {
t.Fatalf("Expected status 504, got: %d", res.StatusCode)
if res.StatusCode != http.StatusBadGateway {
t.Fatalf("Expected status 502, got: %d", res.StatusCode)
}

time.Sleep(time.Second / 4)
Expand Down Expand Up @@ -849,6 +849,24 @@ func TestEndpointErrorHandler(t *testing.T) {
{"error_handler triggered with beresp body", "/not-ok", test.Header{"x": "200", "y": "item1"}, http.StatusTeapot, "unexpected_status"},
{"error_handler triggered with beresp body handled by endpoint", "/not-ok-endpoint", test.Header{"x": "200", "y": "item1"}, http.StatusTeapot, "unexpected_status"},
{"error_handler triggered with beresp body - sequence", "/not-ok-sequence", test.Header{"x": "application/json"}, http.StatusTeapot, "unexpected_status"},

{"unexpected status; handlers for unexpected_status, sequence, endpoint", "/1.1", test.Header{"handled-by": "unexpected_status"}, http.StatusOK, "unexpected_status"},
{"unexpected status; handlers for sequence, endpoint", "/1.2", test.Header{"handled-by": "endpoint"}, http.StatusOK, "unexpected_status"},
{"unexpected status; handler for sequence", "/1.3", test.Header{"handled-by": "sequence"}, http.StatusOK, "unexpected_status"},
{"unexpected status; handler for endpoint", "/1.4", test.Header{"handled-by": "endpoint"}, http.StatusOK, "unexpected_status"},
{"unexpected status; no handlers", "/1.5", test.Header{"couper-error": "endpoint error"}, http.StatusBadGateway, "unexpected_status"},

{"backend timeout; handlers for backend_timeout, backend, sequence, endpoint", "/2.1", test.Header{"handled-by": "backend_timeout"}, http.StatusOK, "backend_timeout"},
{"backend timeout; handlers for backend, sequence, endpoint", "/2.2", test.Header{"handled-by": "backend"}, http.StatusOK, "backend_timeout"},
{"backend timeout; handlers for sequence, endpoint", "/2.3", test.Header{"handled-by": "sequence"}, http.StatusOK, "backend_timeout"},
{"backend timeout; handler for endpoint", "/2.4", test.Header{"handled-by": "endpoint"}, http.StatusOK, "backend_timeout"},
{"backend timeout; no handler", "/2.5", test.Header{"couper-error": "endpoint error"}, http.StatusBadGateway, "backend_timeout"},

{"backend openapi validation; handlers for backend_openapi_validation, backend, sequence, endpoint", "/3.1", test.Header{"handled-by": "backend_openapi_validation"}, http.StatusOK, "backend_openapi_validation"},
{"backend openapi validation; handlers for backend, sequence, endpoint", "/3.2", test.Header{"handled-by": "backend"}, http.StatusOK, "backend_openapi_validation"},
{"backend openapi validation; handlers for sequence, endpoint", "/3.3", test.Header{"handled-by": "sequence"}, http.StatusOK, "backend_openapi_validation"},
{"backend openapi validation; handler for endpoint", "/3.4", test.Header{"handled-by": "endpoint"}, http.StatusOK, "backend_openapi_validation"},
{"backend openapi validation; no handler", "/3.5", test.Header{"couper-error": "endpoint error"}, http.StatusBadGateway, "backend_openapi_validation"},
} {
t.Run(tc.name, func(st *testing.T) {
hook.Reset()
Expand Down

0 comments on commit 51ac083

Please sign in to comment.