Skip to content

[Docs] Fix the fiber context recipe for event loops - #685

Merged
chr-hertel merged 3 commits into
modelcontextprotocol:mainfrom
chr-hertel:fiber-context-follow-ups
Nov 8, 2026
Merged

chr-hertel merged 3 commits into
modelcontextprotocol:mainfrom
chr-hertel:fiber-context-follow-ups

Conversation

@chr-hertel

Copy link
Copy Markdown
Member

Follow-up to #670

  • Docs: Context::getRoot()->activate() with a discarded scope raises a notice as soon as PHP assertions are on - that is an ErrorException in Symfony's debug mode, same setup as [Server] Handler fibers give integrations no hook to carry OpenTelemetry's context #583. The recipe now keeps the scope & detaches it in finally
  • Tests: abandoned streams were only covered with buildStateless(), now all three modes, plus a failing wrap() and a leftover test name

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The event-loop example calls run() on a Builder, causing an undefined-method error; it must call build() first.

1 open issue
What changed in this PR

This follow-up to #670 corrects the OpenTelemetry scope-lifetime recipe for event-loop requests and expands fiber-context regression coverage.

Changes:

  • Retain the activated scope and detach it in finally.
  • Cover abandoned streams and failing wrap() calls across all three server modes.
  • Rename the stdio test to reflect the fiber-context API.
File Description
tests/​Unit/​Server/​Fiber/​FiberContextTest.php Expand cleanup and failure coverage; rename the stdio test.
tests/​Unit/​Fixtures/​RecordingFiberContext.php Allow simulated wrap() failures.
docs/​run/​server-builder.md Document scope retention and cleanup under event loops.

Review effort: Balanced

Comment thread docs/run/server-builder.md Resolved

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are reported for the context-scope cleanup and expanded fiber-context tests.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@chr-hertel
chr-hertel merged commit 3700254 into modelcontextprotocol:main Nov 8, 2026
31 checks passed
@chr-hertel
chr-hertel deleted the fiber-context-follow-ups branch November 8, 2026 13:35
@chr-hertel chr-hertel added the documentation Improvements or additions to documentation label Nov 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants