Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MultipartError always has HTTP status of 500 #2573

Closed
mxcl opened this issue Feb 23, 2021 · 3 comments · Fixed by #2575
Closed

MultipartError always has HTTP status of 500 #2573

mxcl opened this issue Feb 23, 2021 · 3 comments · Fixed by #2575

Comments

@mxcl
Copy link
Contributor

mxcl commented Feb 23, 2021

Since MultipartError does not conform to Abortable (or whatever) it always reports 500, which is incorrect IMO.

It seems to me all should be 400s apart from the nesting one.

This is for decoding submitted multipart data of course.

@siemensikkema
Copy link
Member

@mxcl conforming MultipartError to Abortable is not possible in MultipartKit but could be done in Vapor. However, I'd argue that defining how errors are represented in the applications built on Vapor is best decided by the application developers themselves and can be done by writing the conformance in the application. Choosing which status code, and especially which reason goes with which error is a contentious topic!

Closing this issue since it relates to the Vapor package. Feel free to open an issue on Vapor if you don't agree with my reasoning 🙂

AbortError since multipart-kit is now independent of Vapor the conformance of MultipartError to AbortError

@mxcl
Copy link
Contributor Author

mxcl commented Feb 26, 2021

I have to disagree that the default behavior of a web server should be to misreport status codes, but that’s up to you. Certainly as a user the idea that I have to manually go through every single Error type in Vapor and decide how they should be reported is intimidating.

Thanks.

@0xTim
Copy link
Member

0xTim commented Feb 26, 2021

I agree with @mxcl on this one - it's the equivalent of returning a 500 error when failing on req.content.decode. At best it's ambiguous for clients but also problematic for metrics as well. This does need to be implemented in Vapor however (we do the same with a 404 from Fluent) so I'll transfer this there

@0xTim 0xTim reopened this Feb 26, 2021
@0xTim 0xTim transferred this issue from vapor/multipart-kit Feb 26, 2021
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 a pull request may close this issue.

3 participants