fix: accept HTTP 204 No Content in SendNotification

[satish-karri-glean]

Description

Fixes #700

SendNotification in streamable_http.go currently treats HTTP 204 (No Content) as an error, returning "notification failed with status 204". However, HTTP 204 is a valid success status code per RFC 7231 that indicates the server successfully processed the request but has no content to return. This is semantically appropriate for notification endpoints, as notifications are one-way messages that don't expect a response body.

Many HTTP servers return 204 for successful notification/webhook deliveries. The official TypeScript MCP SDK (@modelcontextprotocol/sdk) already handles this correctly by using response.ok (which accepts any 2xx status).

Changes

• Added http.StatusNoContent (204) to the accepted status codes in SendNotification
• Added TestStreamableHTTP_SendNotification_Accepts204NoContent test

Test plan

• New test TestStreamableHTTP_SendNotification_Accepts204NoContent verifies 204 is accepted
• Existing SendNotification tests continue to pass (202, 401 handling)
• Full test suite passes (go test ./...)

Made with Cursor

Summary by CodeRabbit

• Bug Fixes

  • Notification delivery now treats HTTP 204 No Content as a successful response, improving compatibility with servers that use 204 for successful notifications.

• Tests

  • Added a test confirming that notifications succeed when the server returns 204 No Content.

[coderabbitai]

Walkthrough

SendNotification in client/transport/streamable_http.go now treats HTTP 204 No Content as a successful response (alongside 200 and 202) and centralizes response handling via a switch. A test TestStreamableHTTP_SendNotification_Accepts204NoContent was added to validate 204 acceptance.

Changes

Cohort / File(s)	Summary
Streamable HTTP transport client/transport/streamable_http.go, client/transport/streamable_http_test.go	Accept HTTP 204 No Content in SendNotification using switch-based status handling; preserve OAuth unauthorized fallback behavior; add test TestStreamableHTTP_SendNotification_Accepts204NoContent verifying no error on 204 response.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• fix: Add Accept Header in StreamableHTTP Client #230: Modifies the same SendNotification logic in client/transport/streamable_http.go (related response handling / headers).

Suggested reviewers

• dugenkui03
• pottekkat

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: accepting HTTP 204 No Content as a success response in SendNotification.
Description check	✅ Passed	The PR description comprehensively covers the problem, rationale (RFC 7231), changes made, and test plan; all required template sections are adequately addressed.
Linked Issues check	✅ Passed	The PR fully addresses issue #700: it accepts HTTP 204 as success in SendNotification and includes a test verifying 204 handling, meeting all stated requirements.
Out of Scope Changes check	✅ Passed	All changes are directly related to the stated objective: adding 204 status support and its test, with no unrelated modifications.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

client/transport/streamable_http_test.go (1)

994-1019: Use require.NoError per coding guidelines; add missing transport.Close().

The test uses t.Fatalf for error assertions, but the coding guidelines require using testify/require. Also, transport.Close() is not deferred — other tests in this file (e.g., lines 164, 522) properly close the transport.

♻️ Suggested diff

 func TestStreamableHTTP_SendNotification_Accepts204NoContent(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusNoContent)
 	}))
 	defer server.Close()
 
 	transport, err := NewStreamableHTTP(server.URL)
-	if err != nil {
-		t.Fatalf("Failed to create StreamableHTTP: %v", err)
-	}
+	require.NoError(t, err, "Failed to create StreamableHTTP")
+	defer transport.Close()
 
-	if err := transport.Start(context.Background()); err != nil {
-		t.Fatalf("Failed to start transport: %v", err)
-	}
+	err = transport.Start(context.Background())
+	require.NoError(t, err, "Failed to start transport")
 
 	err = transport.SendNotification(context.Background(), mcp.JSONRPCNotification{
 		JSONRPC: "2.0",
 		Notification: mcp.Notification{
 			Method: "notifications/initialized",
 		},
 	})
 
-	if err != nil {
-		t.Fatalf("SendNotification should accept 204 No Content, got error: %v", err)
-	}
+	require.NoError(t, err, "SendNotification should accept 204 No Content")
 }

As per coding guidelines, **/*_test.go: "Testing: use testify/assert and testify/require".

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}