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

AsyncServerInterceptor fails when non-existent service is invoked #46

Closed
anuraaga opened this issue Nov 8, 2023 · 4 comments
Closed

Comments

@anuraaga
Copy link
Contributor

anuraaga commented Nov 8, 2023

What versions of the following are you using?

  • python: 3.11
  • grpc-interceptor: 0.15.3
  • grpcio: 1.59.2
  • protobuf: 4.24.4

What operating system (Linux, Windows,...) and version?

Mac

What did you do?

Define an AsyncServerInterceptor and query the server with grpcurl.

What did you expect to see?

Service is executed

What did you see instead?

AttributeError: 'NoneType' object has no attribute 'unary_unary'

Anything else we should know about your project / environment?

This happens because next_handler is None here if the requested service doesn't exist

https://github.com/d5h-foss/grpc-interceptor/blob/master/src/grpc_interceptor/server.py#L123

This effectively makes it not possible to use grpcurl which first queries with non-alpha reflection API before falling back to alpha one if it's not implemented. But the internal server error is not handled with that fallback it seems.

@metasyn
Copy link
Contributor

metasyn commented Nov 8, 2023

So interesting - I just found this today - also from using grpcurl. I was able to fix it I think by adding:

if next_handler is None:
    return

right here: https://github.com/d5h-foss/grpc-interceptor/blob/master/src/grpc_interceptor/server.py#L123

Which is what the non-async one does here:
https://github.com/d5h-foss/grpc-interceptor/blob/master/src/grpc_interceptor/server.py#L55-L57

@dan-hipschman
Copy link

Good find, thank you! I've been busy with work but will get to this when I have time. Or happy to review a PR if you'd like to take a stab at it.

@anuraaga
Copy link
Contributor Author

Thanks @dan-hipschman @metasyn I have sent #48 to fix, it was nice seeing there was already a test that just needed to be parameterized

@d5h
Copy link
Collaborator

d5h commented Nov 16, 2023

Thanks again for the fix! The release is out now.

@d5h d5h closed this as completed Nov 16, 2023
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

4 participants