Dealing with 304 response without printing errors

[taichimaeda]

🐛 Bug Report

(A clear and concise description of what the bug is.)

This is not really a bug to be exact, but I've been struggling with the error logs coming from grpc-gateway and was wondering if I could get some help...

I'm using WithForwardResponseOption(forwardResponseOption) to inspect the response message and set the 304 Not Modified status when necessary, but doing so results in the following error:

Failed to write response: http: request method or response status code does not allow body

which comes from this Write call, because you're not allowed to write a response body when it's 304.

This is not the behaviour I'm expecting - a 304 response is not an error, and printing them on every request just makes them noisy.

The way I used to avoid this issue was simply returning an error in the forwardResponseOption function, so that the above Write gets skipped by an early return, and ignoring this error in a custom error handler using theWithErrorHandler.

But unfortuantely, this trick is no longer working for me, since this PR changed grpclog.Infof("Error handling ForwardResponseOptions: %v", err) to grpclog.Errorf("Error handling ForwardResponseOptions: %v", err).
(To be clear, this means it still did print info logs before this PR, but it was ok for me - the error logs are the real problem)

And now I'm out of options...
Is there a good way to get around this, perhaps by modifying handleForwardResponseOptions so that it leaves the printing to the error handler?

To Reproduce

(Write your steps here:)

1. Step 1...
2. Step 2...
3. Step 3...

Expected behavior

(Write what you thought would happen.)

There should be a way to avoid 1. printing error logs and 2. writing a response body, at the same time.

Actual Behavior

(Write what happened. Add screenshots, if applicable.)

The ways I've tried and mentioned above results in printing error logs anyway.

Your Environment

(Environment name, version and operating system.)

[danztran]

I think this might be related to #4483. As described, for now we can consider:

• Setting the environment variable GRPC_GO_LOG_SEVERITY_LEVEL to none to disable all the logs.
• Or creating a new logger that overrides the built-in grpc.Logger.

[johanbrandhorst]

It's interesting to hear from a second user that they're having fundamentally the same issue. It implies to me that there's either a missing feature of missing documentation. Clearly what you are trying to do here is override the entire response, so perhaps the docs that are introduced in that issue and #4564 will help you solve your problem?

[taichimaeda]

perhaps the docs that are introduced in that issue and #4564 will help you solve your problem?

Ah I didn't realise there are docs for that, but my case is slightly different since this still ends up Write-ing the response when the status is 304

Setting the environment variable GRPC_GO_LOG_SEVERITY_LEVEL to none to disable all the logs.

This is not really an option for me as I still need to see other logs...

Or creating a new logger that overrides the built-in grpc.Logger.

This is indeed feasible but seems a bit overkill...

---

My intuition is that since handleForwardResponseOptions already returns an error, it should probably wrap the error from opt and let the receiving handlers like HTTPError handle the logging - is there a particular reason we're avoiding that?

[johanbrandhorst]

I think you make a great point, I'm not sure we need to log that error at all, it'll be returned to the caller and hopefully discovered by the person running the server. Would you be interested in contributing a change to remove that error log?

[taichimaeda-ca]

Would you be interested in contributing a change to remove that error log?

Sure will do in a moment!