InTapHandler: RefusedStream may cause retrying problem

[menghanl]

Currently, InTapHandler error causes a RST_STREAM with RefusedStream

grpc-go/internal/transport/http2_server.go

Lines 363 to 376 in d8ef479

	s.ctx, err = t.inTapHandle(s.ctx, info)
	if err != nil {
	if logger.V(logLevel) {
	logger.Warningf("transport: http2Server.operateHeaders got an error from InTapHandle: %v", err)
	}
	t.controlBuf.put(&cleanupStream{
	streamID: s.id,
	rst: true,
	rstCode: http2.ErrCodeRefusedStream,
	onWrite: func() {},
	})
	s.cancel()
	return false
	}

• RefusedStream MUST NOT be used for the stream if frames are passed to the application layer (https://httpwg.org/specs/rfc7540.html#Reliability)
• This causes retrying problem because the client will retry RefusedStream errors unconditionally.

Since InTapHandler is mostly used for auth checking, a status with code PERMISSION_DENIED would be preferred.

We can also allow users to set the status code, if we want to keep it as a general purpose pre-message interceptor. This could get a bit tricky since InTapHandler is processed in the transport. But since status is now in its own package, this shouldn't be a problem.

[dfawley]

We can also allow users to set the status code
...
But since status is now in its own package

@menghanl - What kind of status code were you thinking? We do a RST_STREAM here, so it must be an http2 error code, no? Or were you thinking this could result in trailers being sent immediately by the server?

[menghanl]

I was thinking we end the stream with a gRPC status, instead of a RST_STREAM. I believe that's also what the other languages do.

[dfawley]

I was thinking we end the stream with a gRPC status

So, send trailers instead of RST_STREAM? (Actually, send trailers, then still send a RST_STREAM after that since the client is presumably not half-closed yet.) That SGTM.

[easwars]

Moving it to @dfawley who has a PR out for this.