fix: fix TestSSE_SendRequest_Timeout flaky test

[everfid-ever]

Description

Fixes the flaky test TestSSE_SendRequest_Timeout/TimeoutWhenServerNeverResponds that fails when the context deadline fires before the internal timer.

Fixes #682

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• MCP spec compatibility implementation
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Code refactoring (no functional changes)
• Performance improvement
• Tests only (no functional changes)
• Other (please describe):

Checklist

• My code follows the code style of this project
• I have performed a self-review of my own code
• I have added tests that prove my fix is effective or that my feature works
• I have updated the documentation accordingly

MCP Spec Compliance

• This PR implements a feature defined in the MCP specification
• Link to relevant spec section: Link text
• Implementation follows the specification exactly

Summary by CodeRabbit

• Tests
  • Enhanced timeout error assertion in SSE transport tests to handle multiple error message formats for improved test robustness.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The test TestSSE_SendRequest_Timeout is modified to accept both "timeout" and "deadline exceeded" error messages as valid timeout indicators, addressing a race condition where either error can occur depending on whether the context deadline or timer fires first.

Changes

Cohort / File(s)	Summary
SSE Timeout Test Assertion client/transport/sse_test.go	Modified error message assertion to accept either "timeout" or "deadline exceeded" by introducing an intermediate errMsg variable for flexible error validation

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

• fix: add timeout for SSE response waiting to prevent indefinite blocking #668: Directly addresses SSE timeout behavior and test expectations, making this assertion change compatible with the underlying timeout handling modifications.

Suggested reviewers

• ezynda3

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the fix - it addresses a flaky test in TestSSE_SendRequest_Timeout with a clear, specific focus.
Description check	✅ Passed	The description follows the template with required sections completed: it identifies the flaky test, references issue #682, and marks the change as a bug fix.
Linked Issues check	✅ Passed	The PR successfully addresses issue #682 by modifying the test assertion to accept both 'timeout' and 'deadline exceeded' error messages, matching the proposed solution.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing the flaky test as specified in issue #682; no unrelated modifications are present.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

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

---

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 498341a and 9bb055a.

📒 Files selected for processing (1)

• client/transport/sse_test.go

🧰 Additional context used

📓 Path-based instructions (2)

**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Order imports: standard library first, then third-party, then local packages (goimports enforces this)
Follow Go naming conventions: exported identifiers in PascalCase; unexported in camelCase; acronyms uppercase (HTTP, JSON, MCP)
Error handling: return sentinel errors, wrap with fmt.Errorf("context: %w", err), and check with errors.Is/As
Prefer explicit types and strongly-typed structs; avoid using any except where protocol flexibility is required (e.g., Arguments any)
All exported types and functions must have GoDoc comments starting with the identifier name; avoid inline comments unless necessary
Functions that are handlers or long-running must accept context.Context as the first parameter
Ensure thread safety for shared state using sync.Mutex and document thread-safety requirements in comments
For JSON: use json struct tags with omitempty for optional fields; use json.RawMessage for flexible/deferred parsing

Files:

• client/transport/sse_test.go

**/*_test.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*_test.go: Testing: use testify/assert and testify/require
Write table-driven tests using a tests := []struct{ name, ... } pattern
Go test files must end with _test.go

Files:

• client/transport/sse_test.go

🧠 Learnings (3)

📓 Common learnings

Learnt from: leavez
Repo: mark3labs/mcp-go PR: 114
File: client/transport/sse.go:137-179
Timestamp: 2025-04-06T10:07:06.685Z
Learning: The SSE client implementation in the MCP-Go project uses a 30-second timeout for reading SSE events to match the behavior of the original implementation before the transport layer refactoring.

📚 Learning: 2025-04-06T10:07:06.685Z

Learnt from: leavez
Repo: mark3labs/mcp-go PR: 114
File: client/transport/sse.go:137-179
Timestamp: 2025-04-06T10:07:06.685Z
Learning: The SSE client implementation in the MCP-Go project uses a 30-second timeout for reading SSE events to match the behavior of the original implementation before the transport layer refactoring.

Applied to files:

• client/transport/sse_test.go

📚 Learning: 2025-10-13T09:35:20.180Z

Learnt from: CR
Repo: mark3labs/mcp-go PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-13T09:35:20.180Z
Learning: Applies to **/*_test.go : Testing: use testify/assert and testify/require

Applied to files:

• client/transport/sse_test.go

🧬 Code graph analysis (1)

client/transport/sse_test.go (1)

client/transport/error.go (1)

• Error (6-8)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: test

🔇 Additional comments (1)

client/transport/sse_test.go (1)

1266-1269: LGTM! Fix correctly addresses the race condition.

The updated assertion now accepts both "timeout" and "deadline exceeded" error messages, which properly handles the race condition between the internal timer and context deadline. The intermediate errMsg variable improves readability, and the OR condition with clear error message makes the test expectation explicit.

---

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.}