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

Tonic client stuck in infinite error status message loop (server side streaming) #681

Closed
MS-Painter opened this issue Jun 15, 2021 · 7 comments · Fixed by #689
Closed

Tonic client stuck in infinite error status message loop (server side streaming) #681

MS-Painter opened this issue Jun 15, 2021 · 7 comments · Fixed by #689
Assignees
Labels
A-tonic C-bug Category: Something isn't working
Milestone

Comments

@MS-Painter
Copy link

MS-Painter commented Jun 15, 2021

Version

tonic = "0.4.3"

tonic-build = "0.4.2" (In build-dependencies)

Platform

Windows 10 x64

Crates

Tonic itself, though might be related to prost -

prost = "0.7.0"
prost-types = "0.7.0"

Might alternatively be related to tokio or more specifically their implementation of broadcast channels -

tokio = { version = "1.6.2", features = ["full"] }
tokio-stream = { version ="0.1.6", features = ["default", "sync"] }

Description

Tried in a project I'm working on, to send on purpose an Error through a tokio broadcast channel.
The channel has receivers broadcasting to server side streams through tonic endpoints.
When sending these intentional errors the client gets stuck in an infinite loop of receiving a different error message after the initial correct error message -

Status { code: Unknown, message: "grpc-status header missing, mapped from 
HTTP status code 200" }

I made a small reproduction repo to help pinpoint the issue -

https://github.com/STRONG-MAD/tonic-server-stream-err-bug-repro

@davidpdrsn davidpdrsn added A-tonic C-bug Category: Something isn't working labels Jun 15, 2021
@LucioFranco
Copy link
Member

The issue is reproducible by sending two Err items in a streaming response. The second (and likely anymore after the first) error messages result in the error message above. Will dig in some more now that I've reproduced it.

@LucioFranco
Copy link
Member

@STRONG-MAD hey! could you verify this fixes your issue? #689

@MS-Painter
Copy link
Author

MS-Painter commented Jun 22, 2021

Tested it out in the reproduction repo. Did not entirely fix the issue.

Now when the err client sends the error.
The first Error gets sent to the trace client as wanted.
But the trace client gets stuck in an infinite loop of getting Ok variant responses that contain just None.

Expected result would be to not get any Ok variant messages after the Err was acknowledged.
And also for the .message() to properly consume that Ok variant message,
And not loop over it endlessly (seems like part of what's happening).

@MS-Painter
Copy link
Author

MS-Painter commented Jun 22, 2021

Similarly in the reproduction repo (not specifically related to the issue opened).
If you close the server side with a trace client still active -
It will loop endlessly with the following Err variant response from .message() -

Status { code: Unknown, message: "h2 protocol error: broken pipe" }

@LucioFranco
Copy link
Member

Actually, the Ok(None) means that the stream is now done. I think if you change your handling to handle that case and exit it should work?

@MS-Painter
Copy link
Author

Yeah it would be ok, was just wondering if that's expected behavior?
And if it's expected behavior for the .message() to continue looping on Ok(None) after it's received once?

@MS-Painter
Copy link
Author

Allegedly, a stream could send an Ok(None) even as a regular message that wasn't related to an error or a done stream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tonic C-bug Category: Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants