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

POST bodies that can't be parsed should return HTTP 415 Unsupported Media Type #767

Open
atheriel opened this issue Feb 8, 2021 · 5 comments
Labels
effort: low < 1 day of work help wanted Solution is well-specified enough that any community member could fix theme: postBody How to consume the postBody value
Milestone

Comments

@atheriel
Copy link
Contributor

atheriel commented Feb 8, 2021

At present, if you decorate a POST endpoint with a parser (e.g. @parser json), and the client does not send you JSON data, you'll just see a message in the console and the endpoint handler will run anyway (even though, presumably, it will fail).

I don't think this is a particularly good way to surface errors to either the client or the operator -- even with the server logs, one can't use a generic message to track down which request had the problem.

Since this is a client error, the client should get a HTTP 4xx response. It turns out there is an existing HTTP status code for exactly this situation: 415 Unsupported Media Type. I'd like to see this returned instead. And, as with #757, I'd ideally like the response body/headers to be controllable.

@meztez
Copy link
Collaborator

meztez commented Feb 8, 2021

I really like your issues. Keep them coming. They are great insights into making plumber more robust.

@atheriel
Copy link
Contributor Author

atheriel commented Feb 8, 2021

Aw, that's kind.

@schloerke schloerke added effort: low < 1 day of work help wanted Solution is well-specified enough that any community member could fix theme: postBody How to consume the postBody value labels Feb 8, 2021
@schloerke schloerke added this to the v1.2.0 milestone Feb 8, 2021
@schloerke
Copy link
Collaborator

Yes, agreed. Keep them coming @atheriel .


As you said, this will need its own set methods like we do with 404 and 405.

I'm wondering if this should enabled by default.

  • If we do, it is a breaking change.
  • If we don't, then authors are running routes with missing / malformed data.

@schloerke
Copy link
Collaborator

As we add more methods (and the become more off the traditional set), I'm wondering if we should have a pr_set_response_code_handler(pr, code, handler) method.

pr_set_404() and pr_set_405() could wrap this function. Adding more response handlers to plumber would just be adding to a switch statement on known handlers.

@schloerke
Copy link
Collaborator

Tagging #809... We should have the ability to remove filters or the ability to overwrite the defaults. Something other than "all" or "up to filter Y"

@schloerke schloerke modified the milestones: v1.2.0, v1.3.0 Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: low < 1 day of work help wanted Solution is well-specified enough that any community member could fix theme: postBody How to consume the postBody value
Projects
None yet
Development

No branches or pull requests

3 participants