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

code.NotFound should return a 404 instead of a 405 #630

Closed
3 tasks done
bobbytables opened this issue Apr 28, 2018 · 8 comments
Closed
3 tasks done

code.NotFound should return a 404 instead of a 405 #630

bobbytables opened this issue Apr 28, 2018 · 8 comments

Comments

@bobbytables
Copy link

Please follow the general troubleshooting steps first:

  • Update your protoc to the latest version
  • Update your copy of grpc-gateway to the latest version from github. with
    git fetch https://github.com/grpc-ecosystem/grpc-gateway master && git reset --hard FETCH_HEAD
  • Delete the protoc-gen-grpc-gateway and protoc-gen-swagger binary from your PATH,
    and install locally built binaries.

Bug reports:

Steps you follow to reproduce the error:

  • Create a grpc endpoint that returns a NotFound (Code 5)
  • Hit grpc-gateway for corresponding endpoint
  • Receive a 405 (Method Not Allowed)

What did you expect to happen instead:

  • The Code 5 should indicate to the grpc-gateway to return a 404, not a Method Not Allowed. In fact, no GET / HEAD request should return this. Per mozilla documentation:

Note: The two mandatory methods, GET and HEAD, must never be disabled and it should not return this type of error.

(https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/405)

What's your theory on why it isn't working:

I'm guessing the code mapping from grpc -> http is a little off.

@achew22
Copy link
Collaborator

achew22 commented Apr 28, 2018

Interesting that you get that result. This is the code where we do that conversion. Are you doing anything non-standard in your setup? I've been using this error code for years in my application without trouble, I'd be interested in running down the problem.

@bobbytables
Copy link
Author

Just realized what it is, it's a HEAD call actually.

@bobbytables
Copy link
Author

I did curl -I which sets the method type to HEAD.

@bobbytables
Copy link
Author

I think I had a possible misunderstanding here, but why does a HEAD request fail it. Technically HEAD should also return an Allowed header per RFC: https://tools.ietf.org/html/rfc2616#section-10.4.6

@achew22
Copy link
Collaborator

achew22 commented Apr 28, 2018

The logic behind this is that we require you to be explicit about the request methods you intend to receive. If you want to receive a HEAD then you will need to use a CustomHttpPattern.

See the documentation line

The custom pattern is used for specifying an HTTP method that is not included in the pattern field, such as HEAD, or "*" to leave the HTTP method unspecified for this rule. The wild-card rule is useful for services that provide content to Web (HTML) clients.

Per the HttpRule documentation.

@achew22
Copy link
Collaborator

achew22 commented Apr 28, 2018

WRT RFC2616 10.4.6, that is another in a long line of things we MUST do according to the RFC, but don't perfectly implement because no one has needed them yet. I would love to merge a PR that adds it.

If compliance with that spec is the name of the game you could also implement a handler that responds to HEAD requests appropriately.

@bobbytables
Copy link
Author

Roger that, sorry for the confusion. Closing.

@achew22
Copy link
Collaborator

achew22 commented Apr 28, 2018

No worries, have a good day!

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

No branches or pull requests

2 participants