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

Peer details not populated when requests fail #1417

Closed
therc opened this issue Aug 3, 2017 · 4 comments
Closed

Peer details not populated when requests fail #1417

therc opened this issue Aug 3, 2017 · 4 comments

Comments

@therc
Copy link
Contributor

therc commented Aug 3, 2017

What version of gRPC are you using?

1.5.1

What version of Go are you using (go version)?

1.8.4

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

Linux

What did you do?

Add a Peer() call option to a call, then send a request that fails somehow (e.g. bad request).

What did you expect to see?

The peer information is populated

What did you see instead?

The peer information is not populated. It seems to work only if the call succeeds.

We looked at this a bit and it seems like the issue is that Peer() is implemented as a deferred "after" function. invoke() will call sendRequest() and, if an error occurs, it will return. The Peer() code will still run, because it was deferred, but the peer details haven't been filled in yet, so the function will just copy a nil value. The callInfo.peer field is only set at the end of recvResponse(), which never gets invoked when a request fails.

Would it be OK to send a PR that sets the peer in sendRequest() instead? Or maybe in both if e.g. AuthInfo can change between the end of sendRequest and recvResponse.

@MakMukhi @jack0

@MakMukhi
Copy link
Contributor

MakMukhi commented Aug 3, 2017

Yes go ahead; the peer is set in a stream's context when it's created so setting it somewhere here should do the trick.

@therc
Copy link
Contributor Author

therc commented Aug 8, 2017

This should be fixed now in HEAD, only waiting for release. Closing.

@therc therc closed this as completed Aug 8, 2017
@menghanl
Copy link
Contributor

@therc This was released in v1.5.2.

@therc
Copy link
Contributor Author

therc commented Aug 14, 2017

@menghanl We just saw that, thanks!

@lock lock bot locked as resolved and limited conversation to collaborators Sep 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants