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

[Enhancement] Improve clarity of Bind invalid Content-Type message #605

Open
1 task done
shawn-hurley opened this issue Feb 21, 2024 · 8 comments
Open
1 task done
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/minor Lowest priority. Possibly useful, but not yet enough support to actually get it done. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@shawn-hurley
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Konveyor version

v0.3.0

Priority

Minor

Current Behavior

When calling the API from a client without setting the Content-Type header the application will error with a "Bind: MIME not supported" error, and the response is empty message 400 to the user.

Expected Behavior

I thought that we could do a couple of things, but some extra info to the user would be helpful or what not. @mansam said he thought y'all wanted it to fall back to application/json if it doesn't exist though.

How Reproducible

Always (Default)

Steps To Reproduce

No response

Environment

- OS:

Anything else?

No response

@shawn-hurley shawn-hurley added kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 21, 2024
@konveyor-ci-bot
Copy link

This issue is currently awaiting triage.
If contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.
The triage/accepted label can be added by org members.

@konveyor-ci-bot konveyor-ci-bot bot added the needs-priority Indicates an issue or PR lacks a `priority/foo` label and requires one. label Feb 21, 2024
@mansam mansam added the priority/minor Lowest priority. Possibly useful, but not yet enough support to actually get it done. label Feb 21, 2024
@konveyor-ci-bot konveyor-ci-bot bot removed the needs-priority Indicates an issue or PR lacks a `priority/foo` label and requires one. label Feb 21, 2024
@mansam mansam added triage/not-reproducible Indicates an issue can not be reproduced as described. needs-priority Indicates an issue or PR lacks a `priority/foo` label and requires one. labels Feb 21, 2024
@konveyor-ci-bot konveyor-ci-bot bot removed the needs-priority Indicates an issue or PR lacks a `priority/foo` label and requires one. label Feb 21, 2024
@mansam mansam added triage/needs-information Indicates an issue needs more information in order to work on it. and removed triage/not-reproducible Indicates an issue can not be reproduced as described. labels Feb 21, 2024
@mansam
Copy link
Collaborator

mansam commented Feb 21, 2024

I don't seem to be able to reproduce this by using curl to POST to the /applications endpoint without explicitly setting a Content-Type header, or when setting an empty Content-Type. I have to explicitly set an invalid Content-Type to get the error message.

curl -XPOST http://localhost:8080/applications -d '{"name":"testapp"}'
# works

curl -XPOST -H"Content-Type:" http://localhost:8080/applications -d '{"name":"testapp"}'
# works

curl -XPOST -H"Content-Type: " http://localhost:8080/applications -d '{"name":"testapp"}'
# works

curl -XPOST -H"Content-Type: xyz" http://localhost:8080/applications -d '{"name":"testapp"}'
# mime type error, as expected

@mansam mansam removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Feb 21, 2024
@konveyor-ci-bot konveyor-ci-bot bot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Feb 21, 2024
@shawn-hurley
Copy link
Contributor Author

Very interesting, I wonder if the javascript Fetch is setting something by default, I think the question would be should we give a reason from the API for the 400?

@shawn-hurley
Copy link
Contributor Author

After searching, it looks like if empty content in fetch, the spec says it sends text/html by default.

I think that if I was an end user of the API, and I didn't have access to the pod logs, I would be pretty confused about what is going wrong. Unless it does exist already and I didn't see it in my testing for whatever reason. if that is the case let's just close this issue?

@mansam
Copy link
Collaborator

mansam commented Feb 22, 2024

The API does return a reason in the message body ({"error":"Bind: MIME not supported."}), but we could revise that to something like {"error":"Bind: Content-Type not supported: text/html"} (where text/html is a placeholder for whatever the offending type is) to help make it more clear what the user might be doing wrong. What do you think?

@shawn-hurley
Copy link
Contributor Author

shawn-hurley commented Feb 22, 2024

+1, now I have to figure out why the body was not printed in my code :)

@mansam mansam changed the title [BUG] when no content-type header there is a 400 response on POST requests [Enhancement] Improve clarity of Bind invalid Content-Type message Feb 22, 2024
@mansam mansam added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed kind/bug Categorizes issue or PR as related to a bug. triage/needs-information Indicates an issue needs more information in order to work on it. labels Feb 22, 2024
@konveyor-ci-bot konveyor-ci-bot bot added needs-kind Indicates an issue or PR lacks a `kind/foo` label and requires one. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 22, 2024
@mansam mansam added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Feb 22, 2024
@konveyor-ci-bot konveyor-ci-bot bot removed the needs-kind Indicates an issue or PR lacks a `kind/foo` label and requires one. label Feb 22, 2024
@jortel
Copy link
Contributor

jortel commented Apr 1, 2024

@shawn-hurley Can this issue be closed?

@mansam
Copy link
Collaborator

mansam commented Apr 1, 2024

@jortel I repurposed it into an enhancement issue to improve the clarity of the error message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/minor Lowest priority. Possibly useful, but not yet enough support to actually get it done. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Status: 📋 Backlog
Development

No branches or pull requests

3 participants