fix(forward): Respond with 413 if request body too large #5630
Conversation
| HttpError::Misconfigured => StatusCode::INTERNAL_SERVER_ERROR.into_response(), | ||
| }, | ||
| Self::Upstream(UpstreamRequestError::SendFailed(e)) => { | ||
| if has_source::<http_body_util::LengthLimitError>(&e) { |
There was a problem hiding this comment.
The length limit error is deeply nested:
Upstream(
SendFailed(
reqwest::Error {
kind: Request,
url: "http://127.0.0.1:56750/api/42/attachment/?sentry_key=f78f4884d0c44e9ead0c80a42e949b50",
source: hyper_util::client::legacy::Error(
SendRequest,
hyper::Error(
User(
Body,
),
reqwest::Error {
kind: Body,
source: Error {
inner: LengthLimitError,
},
},
),
),
},
),
),
There was a problem hiding this comment.
Is there another option with a different/wrapped middleware which makes this less tedious?
There was a problem hiding this comment.
We could possibly make this a little more flat, but the basic problem is that since we're now streaming, the error occurs in the reqwest library while sending the outbound request.
| flate2 = { workspace = true } | ||
| futures = { workspace = true, features = ["async-await"] } | ||
| hashbrown = { workspace = true } | ||
| http-body-util = { workspace = true } |
There was a problem hiding this comment.
I added this only to be able to reference the type. The dependency was already in our tree.
| except Exception: | ||
| # stream might be invalid | ||
| pass |
There was a problem hiding this comment.
Without this, the integration test crashes before we see what relay responds with.
| "/api/42/store/", | ||
| "/api/42/envelope/", | ||
| "/api/666/envelope/", | ||
| "/api/42/attachment/", |
There was a problem hiding this comment.
I changed this back to the original. By using /api/666/envelope/, I obscured the bug that this PR fixes.
| HttpError::Misconfigured => StatusCode::INTERNAL_SERVER_ERROR.into_response(), | ||
| }, | ||
| Self::Upstream(UpstreamRequestError::SendFailed(e)) => { | ||
| if has_source::<http_body_util::LengthLimitError>(&e) { |
There was a problem hiding this comment.
Is there another option with a different/wrapped middleware which makes this less tedious?
2 participants
Follow-up to #5624 (comment)
I verified that the server indeed responds with 502. Fixed by checking the error source in
ForwardError::into_response.