search

TykTechnologies / tyk

Tyk Open Source API Gateway written in Go, supporting REST, GraphQL, TCP and gRPC protocols
Other
9.7k stars 1.09k forks source link

[TT-13404] ReverseProxy does not properly handle received GOAWAY on gRPC (or HTTP/2) connections #6540

Open djablonski-moia opened 1 month ago

djablonski-moia commented 1 month ago

Branch/Environment/Version

Describe the bug When proxying an HTTP/2 request Tyk gateway does not properly handle receiving a GOAWAY frame from the upstream server, but instead just returns an INTERNAL-error, server closed the stream without sending trailers, which does not properly indicate the client to retry.

Reproduction steps Steps to reproduce the behavior:

  1. Configure Tyk gateway to support gRPC streaming (enable HTT/2, flush interval etc.)
  2. Add an API
  3. Connect a client to the API with a backend service that uses gRPC server-side streaming
  4. Gracefully shut down the server (it needs to be implemented in a way to send GOAWAY )
  5. Watch the client error & gateway logs

Actual behavior

When the upstream server sends a GOAWAY, Tyk gateway closes the connection to the client with INTERNAL-error, server closed the stream without sending trailers , but internally logs that it received a GOAWAY.

Expected behavior

The gateway should also send a GOAWAY to the client, so it can handle it properly.

Screenshots/Video

n/a

Logs (debug mode or log file):

gateway-tyk-gateway time="Jul 22 08:24:39" level=error msg="http: proxy error during body copy: http2: server sent GOAWAY and closed the connection; LastStreamID=5, ErrCode=NO_ERROR, debug=\"graceful_stop\"" api_id=my-api api_name="My API" mw=ReverseProxy

Configuration (tyk config file):

(no config file, but environment variables from Helm chart; only those added to the chart's defaults)

TYK_GW_HTTPSERVEROPTIONS_ENABLEHTTP2: true
TYK_GW_HTTPSERVEROPTIONS_FLUSHINTERVAL: 1
TYK_GW_PROXYENABLEHTTP2: true
TYK_GW_LOGLEVEL: info
TYK_GW_HTTPSERVEROPTIONS_READTIMEOUT: 660
TYK_GW_HTTPSERVEROPTIONS_WRITETIMEOUT: 660

Additional context

Looking at the source code, it seems to me that the issue is that the error received from the http2.Transport.RoundTrip() in func (rt *TykRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) (gateway/reverse_proxy.go) just bubbles up to func (p *ReverseProxy) WrappedServeHTTP(rw http.ResponseWriter, req *http.Request, withCache bool) ProxyResponse, where there is no distinction made between HTTP/1.1 and HTTP/2, and therefore no special handling for GOAWAY is possible.