Skip to content

Commit ab07670

Browse files
committed
fix(router): preserve SSE response writer compatibility
1 parent c2d6976 commit ab07670

6 files changed

Lines changed: 73 additions & 16 deletions

File tree

docs-website/router/custom-modules.mdx

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,29 @@ func (m *CustomModule) RouterOnRequest(ctx core.RequestContext, next http.Handle
338338
}
339339
```
340340

341+
#### Streaming responses
342+
343+
Custom response writers used for subscriptions must implement `http.Flusher`.
344+
345+
To retain SSE write timeouts, they must also either implement `SetWriteDeadline(time.Time) error` **or** expose the wrapped response writer:
346+
347+
```go
348+
// required for streaming to function
349+
func (w *headerCapturingWriter) Flush() {
350+
w.ResponseWriter.(http.Flusher).Flush()
351+
}
352+
353+
// Forward write deadlines directly
354+
func (w *headerCapturingWriter) SetWriteDeadline(deadline time.Time) error {
355+
return http.NewResponseController(w.ResponseWriter).SetWriteDeadline(deadline)
356+
}
357+
358+
// or expose the wrapped writer.
359+
func (w *headerCapturingWriter) Unwrap() http.ResponseWriter {
360+
return w.ResponseWriter
361+
}
362+
```
363+
341364
### Request Handler lifecycle
342365

343366
The current module handler allow to intercept and modify request / response subgraphs.

router-tests/subscriptions/http_subscriptions_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,9 @@ func testSSENonFlusherWriter(t *testing.T) {
614614
require.Equal(
615615
t,
616616
`event: next
617-
data: {"errors":[{"message":"subscription response writer does not support flushing"}]}`,
617+
data: {"errors":[{"message":"subscription response writer does not support flushing"}]}
618+
619+
`,
618620
string(body),
619621
)
620622
})

router/core/errors.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,17 @@ func writeRequestErrors(params writeRequestErrorsParams) {
299299
}
300300
params.logger.Error("Error writing response", zap.Error(err))
301301
}
302+
return
303+
}
304+
305+
if wgRequestParams.UseSse {
306+
if _, err := params.writer.Write([]byte("\n\n")); err != nil && params.logger != nil {
307+
if rErrors.IsBrokenPipe(err) {
308+
params.logger.Warn("Broken pipe, error writing response", zap.Error(err))
309+
return
310+
}
311+
params.logger.Error("Error writing response", zap.Error(err))
312+
}
302313
}
303314
}
304315

router/core/graphql_handler.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,7 @@ func (h *GraphQLHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
330330
resolveCtx, writer, writerErr = GetSubscriptionResponseWriter(resolveCtx, r, w, SubscriptionResponseWriterOptions{
331331
ApolloSubscriptionMultipartPrintBoundary: h.apolloSubscriptionMultipartPrintBoundary,
332332
SSEWriteTimeout: h.sseServerWriteTimeout,
333+
Logger: reqCtx.logger,
333334
})
334335
if writerErr != nil {
335336
reqCtx.logger.Error("unable to get subscription response writer", zap.Error(writerErr))

router/core/subscription_response_writer.go

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
"github.com/wundergraph/astjson"
1616
"github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve"
17+
"go.uber.org/zap"
1718
)
1819

1920
const (
@@ -37,6 +38,7 @@ type withFlushWriter interface {
3738
type SubscriptionResponseWriterOptions struct {
3839
ApolloSubscriptionMultipartPrintBoundary bool
3940
SSEWriteTimeout time.Duration
41+
Logger *zap.Logger
4042
}
4143

4244
type HttpFlushWriter struct {
@@ -51,6 +53,7 @@ type HttpFlushWriter struct {
5153
buf *bytes.Buffer
5254
firstMessage bool
5355
sseWriteTimeout time.Duration
56+
logger *zap.Logger
5457
// apolloSubscriptionMultipartPrintBoundary if set to true will send the multipart boundary at the end of the message to allow
5558
// misbehaving client (like apollo client) to read the message just sent before the next one or the heartbeat
5659
apolloSubscriptionMultipartPrintBoundary bool
@@ -189,16 +192,25 @@ func (f *HttpFlushWriter) Flush() (err error) {
189192

190193
func (f *HttpFlushWriter) writeAndFlushSSE(write func() error) (err error) {
191194
if f.sseWriteTimeout > 0 {
192-
if err := f.responseControl.SetWriteDeadline(time.Now().Add(f.sseWriteTimeout)); err != nil {
193-
// Failing closed prevents a response writer without deadline support from
194-
// reintroducing an unbounded shared-trigger stall.
195-
return fmt.Errorf("set SSE write deadline: %w", err)
196-
}
197-
defer func() {
198-
if clearErr := f.responseControl.SetWriteDeadline(time.Time{}); clearErr != nil {
199-
err = errors.Join(err, fmt.Errorf("clear SSE write deadline: %w", clearErr))
195+
if deadlineErr := f.responseControl.SetWriteDeadline(time.Now().Add(f.sseWriteTimeout)); deadlineErr != nil {
196+
if !errors.Is(deadlineErr, http.ErrNotSupported) {
197+
return fmt.Errorf("set SSE write deadline: %w", deadlineErr)
198+
}
199+
200+
f.sseWriteTimeout = 0
201+
if f.logger != nil {
202+
f.logger.Warn(
203+
"SSE write timeout disabled because response writer does not support write deadlines",
204+
zap.Error(deadlineErr),
205+
)
200206
}
201-
}()
207+
} else {
208+
defer func() {
209+
if clearErr := f.responseControl.SetWriteDeadline(time.Time{}); clearErr != nil {
210+
err = errors.Join(err, fmt.Errorf("clear SSE write deadline: %w", clearErr))
211+
}
212+
}()
213+
}
202214
}
203215

204216
if err := write(); err != nil {
@@ -231,6 +243,7 @@ func GetSubscriptionResponseWriter(ctx *resolve.Context, r *http.Request, w http
231243
buf: &bytes.Buffer{},
232244
firstMessage: true,
233245
sseWriteTimeout: opts.SSEWriteTimeout,
246+
logger: opts.Logger,
234247
apolloSubscriptionMultipartPrintBoundary: opts.ApolloSubscriptionMultipartPrintBoundary,
235248
}
236249

router/core/subscription_response_writer_test.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212
"github.com/stretchr/testify/assert"
1313
"github.com/stretchr/testify/require"
1414
"github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve"
15+
"go.uber.org/zap"
16+
"go.uber.org/zap/zaptest/observer"
1517
)
1618

1719
type deadlineRecorder struct {
@@ -244,16 +246,21 @@ func TestGetSubscriptionResponseWriter(t *testing.T) {
244246
assert.Nil(t, writer)
245247
})
246248

247-
t.Run("fails closed when an SSE deadline is configured but unsupported", func(t *testing.T) {
249+
t.Run("disables the SSE timeout when write deadlines are unsupported", func(t *testing.T) {
248250
recorder := httptest.NewRecorder()
249251
req := httptest.NewRequest(http.MethodPost, "/graphql", nil)
250252
req.Header.Set("Accept", sseMimeType)
253+
logCore, logs := observer.New(zap.WarnLevel)
251254

252-
_, writer, err := GetSubscriptionResponseWriter(resolve.NewContext(context.Background()), req, recorder, SubscriptionResponseWriterOptions{SSEWriteTimeout: time.Second})
253-
require.Error(t, err)
254-
assert.ErrorIs(t, err, http.ErrNotSupported)
255-
assert.ErrorContains(t, err, "set SSE write deadline")
256-
assert.Nil(t, writer)
255+
_, writer, err := GetSubscriptionResponseWriter(resolve.NewContext(context.Background()), req, recorder, SubscriptionResponseWriterOptions{
256+
SSEWriteTimeout: time.Second,
257+
Logger: zap.New(logCore),
258+
})
259+
require.NoError(t, err)
260+
require.NotNil(t, writer)
261+
require.NoError(t, writer.Heartbeat())
262+
assert.True(t, recorder.Flushed)
263+
assert.Equal(t, 1, logs.FilterMessage("SSE write timeout disabled because response writer does not support write deadlines").Len())
257264
})
258265

259266
t.Run("does not require deadline support when the timeout is disabled", func(t *testing.T) {

0 commit comments

Comments
 (0)