Skip to content

fix(errors): make the configuration error catchable by an error_handler - #1055

Open
paoValle wants to merge 1 commit into
coupergateway:mainfrom
paoValle:fix/empty-origin-error-handler
Open

paoValle wants to merge 1 commit into
coupergateway:mainfrom
paoValle:fix/empty-origin-error-handler

Conversation

@paoValle

Copy link
Copy Markdown

Closes #983.

errors.Configuration was never registered in errors.Definitions, so the generator left it out of both the types set and SuperTypesMapsByContext. The first omission made a configuration that names it fail at load with error type is unknown: "configuration"; the second meant the catch-all * — which is expanded into an enumerated subtype list — could not cover it either, so the failure could only reach the client as a generic 500.

Registering Configuration in the api and endpoint contexts and regenerating errors/types_generated.go makes it catchable by name and through */label-less handlers. The error-type tables in docs/website/content/configuration/error-handling.md gain the row in both contexts, as the previous commits touching errors/type_defintions.go did.

Verification

$ go test -v -count=1 ./server/ -run TestErrorHandler_Configuration
--- PASS: TestErrorHandler_Configuration (0.01s)
    --- PASS: TestErrorHandler_Configuration//configuration (0.00s)
    --- PASS: TestErrorHandler_Configuration//wildcard (0.00s)
ok  github.com/coupergateway/couper/server  0.410s

$ go test -count=1 -race ./server/ -run 'TestErrorHandler|TestAccessControl_ErrorHandler|Test_ErroneousBackendResponse'   ok
$ go test -count=1 ./errors/... ./config/...                                                                              ok
$ go run errors/generate/types.go   # no diff: the generated file is in sync

Mutation checks: reverting errors/ makes the new test fail at load with 10_couper.hcl:9,19-34: error type is unknown: "configuration"; removing "configuration" from the "*" lists alone turns the wildcard subtest red (want 403, got 500), so both halves are load-bearing.

go test ./... has two failures on this machine (gzip/Content-Length, TestBackend_Compression_ModifyAcceptEncoding and TestHTTPServer_NoGzipForSmallContent); they fail identically on the base commit with go 1.27 against the pinned 1.26.9.

One thing for review

This makes every per-request errors.Configuration catchable in api/endpoint — not only the empty-origin case — which is a visible change for existing catch-all handlers, and error_handler "backend" still does not catch it. If configuration errors are meant to hang off the backend super type instead, say so and I will redo it that way. (Unmapped kinds such as evaluation, server and proxy stay uncatchable; that is pre-existing and untouched here.)

A backend origin that evaluates to an empty string raises a configuration
error per request, but the type was missing from the generated error type set.
`error_handler "configuration"` was rejected at startup and a catch-all
handler never saw the error, so the empty-origin guard pattern could only
reach the client as a generic 500 response.

Registering the type for the api and endpoint contexts makes it catchable by
name and adds it to the `*` super-type.

Closes coupergateway#983
@paoValle

Copy link
Copy Markdown
Author

Hi — a quick check on this proposal: is it something you would like to take? One thing worth knowing: no workflow has ever run on the branch, so nothing has exercised the change yet.

If there is no interest I will go ahead and close it in ten days or so, no worries at all — I would just rather keep the queue here tidy than leave it sitting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No error_handler can catch the configuration error raised when a backend origin evaluates to an empty string

1 participant