Merge branch 'main' into feat(router)--enable-fail-open-on-rate-limit-redis-unavailability

Author: Sam-tesouro

router-tests/error_handling_test.go

@@ -3,6 +3,7 @@ package integration
 import (
 	"cmp"
 	"encoding/json"
+	"github.com/wundergraph/cosmo/router/core"
 	"net/http"
 	"slices"
 	"testing"
@@ -1102,4 +1103,67 @@ func TestErrorPropagation(t *testing.T) {
 			require.Equal(t, `{"errors":[{"message":"Unauthorized","locations":[{"line":1,"column":1}],"extensions":{"code":"UNAUTHORIZED"}}],"data":{"employees":null}}`, res.Body)
 		})
 	})
+
+	t.Run("validate error when a non subscription multipart is printed", func(t *testing.T) {
+		t.Parallel()
+
+		testenv.Run(t, &testenv.Config{
+			NoRetryClient: true,
+			RouterOptions: []core.Option{
+				core.WithEngineExecutionConfig(config.EngineExecutionConfiguration{
+					EnableNetPoll:          true,
+					EnableSingleFlight:     true,
+					MaxConcurrentResolvers: 1,
+				}),
+				core.WithSubgraphRetryOptions(false, 0, 0, 0),
+			},
+		}, func(t *testing.T, xEnv *testenv.Environment) {
+			resp, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
+				Header: map[string][]string{
+					"service-name": {"service-name"},
+					"accept":       {"multipart/mixed;deferSpec=20220824"},
+				},
+				Query: `query employees { employees { ide } }`, // Missing closing bracket
+			})
+
+			expected := "--graphql\r\n" +
+				"Content-Type: application/json\r\n" +
+				"\r\n" +
+				"{\"errors\":[{\"message\":\"field: ide not defined on type: Employee\",\"path\":[\"query\",\"employees\"]}]}\r\n" +
+				"--graphql--"
+			require.Equal(t, expected, resp.Body)
+			require.NoError(t, err)
+		})
+	})
+
+	t.Run("validate the error format when a subscription multipart is printed", func(t *testing.T) {
+		t.Parallel()
+
+		testenv.Run(t, &testenv.Config{
+			NoRetryClient: true,
+			RouterOptions: []core.Option{
+				core.WithEngineExecutionConfig(config.EngineExecutionConfiguration{
+					EnableNetPoll:          true,
+					EnableSingleFlight:     true,
+					MaxConcurrentResolvers: 1,
+				}),
+				core.WithSubgraphRetryOptions(false, 0, 0, 0),
+			},
+		}, func(t *testing.T, xEnv *testenv.Environment) {
+			resp, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
+				Header: map[string][]string{
+					"service-name": {"service-name"},
+					"accept":       {"multipart/mixed;deferSpec=20220824"},
+				},
+				Query: `subscription employees { employees { ide } }`, // Missing closing bracket
+			})
+
+			expected := "--graphql\r\n" +
+				"Content-Type: application/json\r\n" +
+				"\r\n" +
+				"{\"payload\":{\"errors\":[{\"message\":\"field: employees not defined on type: Subscription\",\"path\":[\"subscription\"]}]}}"
+			require.Equal(t, expected, resp.Body)
+			require.NoError(t, err)
+		})
+	})
 }

router-tests/events/nats_events_test.go

@@ -534,7 +534,7 @@ func TestNatsEvents(t *testing.T) {
 
 					// Read the first part
 
-					expected := "\r\n--graphql\nContent-Type: application/json\r\n\r\n{\"payload\":{\"data\":{\"countFor\":0}}}\n"
+					expected := "\r\n--graphql\r\nContent-Type: application/json\r\n\r\n{\"payload\":{\"data\":{\"countFor\":0}}}\n"
 					read := make([]byte, len(expected))
 					_, err = reader.Read(read)
 					assert.NoError(t, err)
@@ -583,7 +583,7 @@ func TestNatsEvents(t *testing.T) {
 
 					// Read the first part
 
-					expected := "\r\n--graphql\nContent-Type: application/json\r\n\r\n{\"payload\":{\"data\":{\"countFor\":0}}}\n\r\n--graphql"
+					expected := "\r\n--graphql\r\nContent-Type: application/json\r\n\r\n{\"payload\":{\"data\":{\"countFor\":0}}}\n\r\n--graphql"
 					read := make([]byte, len(expected))
 					_, err = reader.Read(read)
 					assert.NoError(t, err)

router/CHANGELOG.md

@@ -4,6 +4,12 @@ Binaries are attached to the github release otherwise all images can be found [h
 All notable changes to this project will be documented in this file.
 See [Conventional Commits](https://conventionalcommits.org) for commit guidelines.
 
+## [0.189.1](https://github.com/wundergraph/cosmo/compare/[email protected]@0.189.1) (2025-03-10)
+
+### Bug Fixes
+
+* resolve multipart not working properly for some clients ([#1650](https://github.com/wundergraph/cosmo/issues/1650)) ([8b2d35a](https://github.com/wundergraph/cosmo/commit/8b2d35a72022957e13bf158092ba32d58b757623)) (@SkArchon)
+
 # [0.189.0](https://github.com/wundergraph/cosmo/compare/[email protected]@0.189.0) (2025-03-07)
 
 ### Features

router/core/errors.go

@@ -212,8 +212,10 @@ func writeRequestErrors(r *http.Request, w http.ResponseWriter, statusCode int,
 				return
 			}
 		} else if wgRequestParams.UseMultipart {
+			requestContext := getRequestContext(r.Context())
+
 			// Handle multipart error response
-			if err := writeMultipartError(w, requestErrors, requestLogger); err != nil {
+			if err := writeMultipartError(w, requestErrors, requestContext.operation.opType); err != nil {
 				if requestLogger != nil {
 					requestLogger.Error("error writing multipart response", zap.Error(err))
 				}
@@ -240,7 +242,11 @@ func writeRequestErrors(r *http.Request, w http.ResponseWriter, statusCode int,
 }
 
 // writeMultipartError writes the error response in a multipart format with proper boundaries and headers.
-func writeMultipartError(w http.ResponseWriter, requestErrors graphqlerrors.RequestErrors, requestLogger *zap.Logger) error {
+func writeMultipartError(
+	w http.ResponseWriter,
+	requestErrors graphqlerrors.RequestErrors,
+	operationType OperationType,
+) error {
 	// Start with the multipart boundary
 	prefix := GetWriterPrefix(false, true, true)
 	if _, err := w.Write([]byte(prefix)); err != nil {
@@ -257,12 +263,21 @@ func writeMultipartError(w http.ResponseWriter, requestErrors graphqlerrors.Requ
 		return err
 	}
 
-	resp, err := wrapMultipartMessage(responseBytes)
+	isSubscription := operationType == "subscription"
+	resp, err := wrapMultipartMessage(responseBytes, isSubscription)
 	if err != nil {
 		return err
 	}
 
-	resp = append(resp, '\n')
+	// The multipart spec requires us to use both CRLF (\r and \n) characters together. Since we didn't do this
+	// before, some clients that rely on both CR and LF strictly to parse blocks were broken and not parsing our
+	// multipart chunks correctly. With this fix here (and in a few other places) the clients are now working.
+	if isSubscription {
+		resp = append(resp, '\r', '\n')
+	} else {
+		resp = append(resp, []byte("\r\n--graphql--")...)
+	}
+
 	if _, err := w.Write([]byte(resp)); err != nil {
 		return err
 	}

router/core/flushwriter.go

@@ -88,7 +88,7 @@ func (f *HttpFlushWriter) Flush() (err error) {
 	}
 	if f.multipart && len(resp) > 0 {
 		var err error
-		resp, err = wrapMultipartMessage(resp)
+		resp, err = wrapMultipartMessage(resp, true)
 		if err != nil {
 			return err
 		}
@@ -156,23 +156,27 @@ func GetSubscriptionResponseWriter(ctx *resolve.Context, r *http.Request, w http
 	return ctx, flushWriter, true
 }
 
-func wrapMultipartMessage(resp []byte) ([]byte, error) {
+func wrapMultipartMessage(resp []byte, wrapPayload bool) ([]byte, error) {
 	if string(resp) == heartbeat {
 		return resp, nil
 	}
 
-	// Per the Apollo docs, multipart messages are supposed to be json, wrapped in ` "payload"`
-	a, err := astjson.Parse(`{"payload": {}}`)
+	respValuePreMerge, err := astjson.ParseBytes(resp)
 	if err != nil {
 		return nil, err
 	}
 
-	b, err := astjson.ParseBytes(resp)
+	if !wrapPayload {
+		return respValuePreMerge.MarshalTo(nil), nil
+	}
+
+	// Per the Apollo docs, multipart messages are supposed to be json, wrapped in `"payload"`
+	// for subscriptions
+	payloadWrapper, err := astjson.Parse(`{"payload": {}}`)
 	if err != nil {
 		return nil, err
 	}
-
-	respValue, _, err := astjson.MergeValuesWithPath(a, b, "payload")
+	respValue, _, err := astjson.MergeValuesWithPath(payloadWrapper, respValuePreMerge, "payload")
 	if err != nil {
 		return nil, err
 	}
@@ -259,7 +263,7 @@ func GetWriterPrefix(sse bool, multipart bool, firstMessage bool) string {
 		if firstMessage {
 			messageStart = multipartStart
 		}
-		flushBreak = messageStart + "\nContent-Type: " + jsonContent + "\r\n\r\n"
+		flushBreak = messageStart + "\r\nContent-Type: " + jsonContent + "\r\n\r\n"
 	}
 
 	return flushBreak

router/package.json

@@ -1,6 +1,6 @@
 {
   "name": "router",
-  "version": "0.189.0",
+  "version": "0.189.1",
   "private": true,
   "description": "Placeholder package to simplify versioning and releasing with lerna.",
   "keywords": [