Update 413 status code error handling#2813
Conversation
Signed-off-by: Stef Graces <stefgraces@hotmail.com>
| @@ -358,6 +358,9 @@ func logAndServeError(w http.ResponseWriter, r *http.Request, err error) { | |||
| if parseErr, ok := embeddedErr.(*errors.ParseError); ok && go_errors.As(parseErr.Reason, &maxBytesError) { | |||
There was a problem hiding this comment.
Not sure if this if statement actually ever works anymore, or how to test it.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2813 +/- ##
===========================================
- Coverage 66.46% 26.33% -40.13%
===========================================
Files 92 191 +99
Lines 9258 20264 +11006
===========================================
- Hits 6153 5336 -817
- Misses 2359 14091 +11732
- Partials 746 837 +91
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Found a test for this as well, but I'm unsure if these are still being run in the pipeline: https://github.com/sigstore/rekor/blob/main/cmd/rekor-server/e2e_test.go#L370 @bobcallaway @dlorenc @lukehinds let me know if you guys have any feedback. Thanks in advance. |
|
#2640 seems like this would have helped to address this, i dont know that cosign has picked it up yet. also that test case is still runs but only tests the status code, not the actual message printed |
|
Hi @bobcallaway, Thanks for your response. I feel like this is a rekor issue though. In the main branch the server currently returns a 500 error, instead of a 413 when the payload is too large, hence the PR and my question if the e2e test is being run, as I would assume this test would fail. It's not just about the message. See the following logging for reference: |
|
And after running with the code changes in this PR: |
looks like that test case does run regularly and passes but it tests a different endpoint (SearchLogQuery) which means this is more of a missing coverage situation. |
86f9afd to
e69136a
Compare
|
Hi @bobcallaway I added an e2e test case for this. |
Signed-off-by: Stef Graces <stefgraces@hotmail.com>
e69136a to
7eddeda
Compare
Signed-off-by: Stef Graces <stefgraces@hotmail.com>
Signed-off-by: Stef Graces <stefgraces@hotmail.com>
Summary
Fixes #2808
Error handling for 413 errors seems to not properly return an error (anymore). When trying to create an entry with too large of a body the following generic errors were returned:
This PR solves it by returning the proper http error code and a useful message:
This was tested by running in debug mode with all dependencies running in docker via the docker compose and using cosign v2.6.2:
Release Note
Documentation
N/A