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

Is there any place proper to log the grpc or http error info? #887

Open
inevity opened this issue Jan 12, 2022 · 3 comments
Open

Is there any place proper to log the grpc or http error info? #887

inevity opened this issue Jan 12, 2022 · 3 comments
Labels
A-tonic C-bug Category: Something isn't working

Comments

@inevity
Copy link

inevity commented Jan 12, 2022

Bug Report

Just use tower-http trace's new_for_grpc.
And do server timeout test : client pass grpc-timeout 1H, and server build with timeout 1ms, here use tonic::server::timeout, not tower timeout.
Then no any log in the server tonic , even enable the tower-http trace and trace level.

Can hack the tonic service to log the time expired error.
But how can get the error info log by the middleware or get current rpc status from my service code?

Version

0.6.2

Platform

Crates

Description

@davidpdrsn
Copy link
Member

The middleware probably gets applied in the wrong order which is a bug.

The only workaround for now would be for you to write to your own timeout middleware (perhaps use tower's) and apply it after TraceLayer.

Adding this to the 0.7 milestone but I don't it'll require breaking changes to fix.

@davidpdrsn davidpdrsn added A-tonic C-bug Category: Something isn't working labels Jan 13, 2022
@davidpdrsn davidpdrsn added this to the 0.7 milestone Jan 13, 2022
@inevity
Copy link
Author

inevity commented Jan 15, 2022

  1. tonic::transport::Server::builder().layer(mylayer).timeout().add_service....
    client got time expired and code Cancelled.
    but tracelayer cannot capture any error.
  2. mylayer: tower::ServiceBuilder::new().timeout().layer(TraceLayer::new())
    then add this mylayer to the tonic Server::builder()
    h2 log error, but tracelayer cannot capture any error
    client got transport error, code: Unknown
  3. mylayer: tower::ServiceBuilder::new().layer(TraceLayer::new())..timeout()
    tracelayer on_failure callback capture this error:response failed classification=Error: request timed out latency=18 ms
    h2 log same error as 2's log2: response failed classification=Error: request timed out latency=18 ms
    client got transport error, code: Unknown

And use the tower timeout , however, we as client got transport error, though tracelayer on-failure can correctly classifiy as the request timed out error .

So we need fix the tower timeout middleware or add a tower-http timeout middleware to support return time expired errror message. And deprecate the tonic::Server timeout method.

@inevity
Copy link
Author

inevity commented Jan 17, 2022

Just move the tonic::transport::Service::timeout code to tower-http. Now run ok .
Some questions:

  • If use the TimeoutExpired Error, which show as a h2 error in the tracelayer log , the grpc client parse it message as 'transport error', Unknow status code. Did not find where the server code send/return this error?
  • How return correct error/status code with message? which method we prefer?
    1. Can we use use tonic's status struct in the tower-http's timeout middleware code which i just create to reply a grcp status?

    2. Should we create a tower-http middleware map_response_error to convert this h2/or the wrong grcp status to correct status code(Canceld, time expired)?

    3. use tonic client to convert this status to correct.

Retest on the master of tonic now, preview on 0.6.2:
Even have fix(transport): Correctly map hyper errors #629, fix(tonic): Expose h2 error instead of reason #883, found the error still cannot convert by the try_from_error.

Error is 'tonic::transport::Error(Transport, hyper::Error(Http2, Error { kind: Reset(StreamId(1), INTERNAL_ERROR, Remote) }))', try_from_error process result: cannot not parse as grpc error, cannot parse as h2 error, can not parse by the source chain, then return the Error above.

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

No branches or pull requests

3 participants