Skip to content

fix: fail fast on Mongo socket receive timeouts#42

Merged
loks0n merged 4 commits into
mainfrom
fix/receive-fail-fast
Jul 14, 2026
Merged

fix: fail fast on Mongo socket receive timeouts#42
loks0n merged 4 commits into
mainfrom
fix/receive-fail-fast

Conversation

@ChiragAgg5k

@ChiragAgg5k ChiragAgg5k commented Jul 14, 2026

Copy link
Copy Markdown
Member

What does this PR do?

Fails fast when a Swoole Mongo socket recv() times out, instead of retrying up to 10 000 times and turning a single ~30s socket timeout into a multi-minute hang.

This was found while investigating Appwrite CI flake DocumentsDBCustomServerTest::testTimeout (create of ~12MB documents under load). Appwrite never got an HTTP response; the e2e client hit:

Exception: Operation timed out after 12000x milliseconds with 0 bytes received

Appwrite logs during the hang showed the library spinning / then failing late:

Utopia\Mongo\Exception: Receive timeout: no data received within reasonable time
Swoole\Client::send(): ... Broken pipe

Root cause

Client::receive() treated recv() === false (and empty string) as “try again” with adaptive sleep, up to maxAttempts = 10000. The Swoole client already has 'timeout' => 30, so each failed recv() can wait ~30s. Retrying that path multiplies into hangs that exceed Appwrite’s HTTP client 120s curl timeout — the API never responds.

Relevant before behavior:

$attempts = 0;
$maxAttempts = 10000;
$sleepTime = 100;

do {
    $chunk = @$this->client->recv();

    if ($chunk === false || $chunk === '') {
        $attempts++;
        if ($attempts >= $maxAttempts) {
            throw new Exception('Receive timeout: no data received within reasonable time');
        }
        // sleep / backoff, continue...
    }

    // any partial byte reset $attempts = 0
} while (true);

Fix

  1. recv() === false → throw immediately (socket wait already elapsed; do not multiply it).
  2. Wall-clock deadline = $receiveTimeout (30s), shared with the Swoole 'timeout' option.
  3. '' + nonzero errCode → treat as timeout (some Swoole builds return empty string on timeout instead of false).
  4. Transient empty reads with errCode === 0 still yield briefly until the deadline.
  5. Clearer send errors include errCode after reconnection failure.

Relevant after behavior:

$deadline = \microtime(true) + $this->receiveTimeout;

do {
    if (\microtime(true) >= $deadline) {
        throw new Exception('Receive timeout: no data received within reasonable time');
    }

    $chunk = @$this->client->recv();
    $errCode = $this->client->errCode ?? 0;

    if ($chunk === false) {
        throw new Exception(
            'Receive timeout: no data received within reasonable time'
            . ($errCode !== 0 ? " (errCode={$errCode})" : '')
        );
    }

    if ($chunk === '' && $errCode !== 0) {
        throw new Exception(
            'Receive timeout: no data received within reasonable time'
            . " (errCode={$errCode})"
        );
    }

    if ($chunk === '') {
        // brief yield until deadline
        continue;
    }
    // ... assemble response ...
} while (true);

Under load, Appwrite now surfaces fail-fast errors like:

Receive timeout: no data received within reasonable time (errCode=11)

instead of silently blocking until the HTTP client gives up.

How we reproduced (Appwrite + this driver)

Local OrbStack stack, CPU-capped to force Mongo contention:

docker update --cpus=0.5 appwrite
docker update --cpus=0.5 appwrite-mongodb

# 3 concurrent DocumentsDBCustomServerTest::testTimeout loops × 5 iters
# (each creates 10 × ~12MB documents, then asserts query timeout → 408)

Stress results

Run OK Client 120s hang (0 bytes received) Other
Unpatched 3 3+ (then cascade) broken pipes / account 500s
Patch v1 (recv false fail-fast only) 11 4
Patch v2 (this PR: + empty/errCode + send errCode) 9 2 3× invalid JSON, 1× 500 ≠ 408

What this fixes: the library no longer multiplies one socket timeout into a multi-minute receive loop. Failures show up as Mongo Receive timeout (errCode=11) within ~one socket timeout budget.

What this does not fully eliminate: under extreme 0.5 CPU + concurrent 12MB creates, some requests can still starve past curl’s 120s (worker/CPU starvation) or return truncated bodies. That class of flake still wants a lighter Appwrite testTimeout setup and/or mapping Mongo receive timeouts → HTTP 408 on the Appwrite side (DATABASE_TIMEOUT).

Alone / unloaded, testTimeout completes in ~10–14s.

Test plan

  • CI: existing utopia-php/mongo unit/integration suite
  • Local Appwrite stress (above) with patched vendor Client.php copied into running container
  • Confirmed fail-fast log line Receive timeout ... (errCode=11) after patch
  • Optional follow-ups (separate repos):
    • Appwrite: map Mongo receive timeout → DATABASE_TIMEOUT (408)
    • Appwrite: lighten DatabasesBase::testTimeout document payload / count

Notes / BC

  • Default timeout remains 30s (same as previous hardcoded Swoole timeout).
  • Behavior change: callers that previously hung for minutes on a dead/slow socket will now get an exception roughly within one receive timeout. That is intentional.
  • Inserts/writes still do not pass maxTimeMS via the database adapter; this PR only fixes the wire-protocol receive loop, not query maxTimeMS semantics.

Summary by CodeRabbit

  • Bug Fixes
    • Improved connection receive timeout handling with a consistent, configurable deadline.
    • Prevented empty, non-error socket reads from causing premature failures.
    • Enhanced error reporting when sending fails after a reconnection attempt.
    • Ensured receive operations fail promptly on socket errors and timeouts.

Stop retrying recv() false up to 10k times after the socket timeout already
elapsed, which could hang Appwrite requests past the HTTP client's 120s limit.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@ChiragAgg5k, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 42 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8abbdb39-01c2-4969-9ba7-523aa9616bde

📥 Commits

Reviewing files that changed from the base of the PR and between bd1a4cc and 9662b1e.

📒 Files selected for processing (1)
  • src/Client.php
📝 Walkthrough

Walkthrough

Changes

Socket timeout handling

Layer / File(s) Summary
Configurable receive timeout
src/Client.php
Adds a receive timeout property and applies it to the Swoole client socket option.
Deadline-based receive flow
src/Client.php
Replaces retry/backoff attempts with a fixed wall-clock deadline, immediate socket error handling, and brief yields for transient empty reads.
Reconnect failure reporting
src/Client.php
Includes the Swoole error code when the second send attempt fails with a non-zero code.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: abnegate, fogelito

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant SwooleClient
  participant Scheduler
  Client->>SwooleClient: recv()
  SwooleClient-->>Client: data or transient empty read
  Client->>Client: Check wall-clock deadline
  Client->>Scheduler: Sleep briefly
  Scheduler-->>Client: Resume before deadline
  Client->>SwooleClient: recv() again
  SwooleClient-->>Client: Data or receive error
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: Mongo socket receives now fail fast on timeouts.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/receive-fail-fast

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

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces a 10,000-attempt retry loop in receive() with a fail-fast approach: recv() === false now throws immediately, and a wall-clock idle deadline (reset on every real chunk) guards against silent socket stalls. A configurable $timeout constructor parameter (default 30 s) is threaded through to both the Swoole socket option and the idle deadline.

  • Fail-fast receive: recv() returning false throws at once instead of multiplying one socket-level wait into a multi-minute hang; the idle deadline (not a fixed wall-clock budget) resets on each arriving chunk so large responses can still complete.
  • Configurable timeout: $timeout is now an optional constructor argument with validation, resolving the previously hardcoded 30 s value.
  • Improved send error reporting: The first non-zero errCode is preserved across the reconnect-and-retry path and surfaced in the exception message.

Confidence Score: 5/5

Safe to merge. The core logic change is correct: recv()===false now throws immediately, the idle deadline resets on each real chunk (so large responses complete), and the new constructor parameter is backward-compatible.

The fix eliminates the root cause of the multi-minute hang correctly. The deadline-reset-on-chunk approach properly handles large multi-chunk responses without imposing a hard wall-clock budget from the first byte, addressing the concern raised in the previous review round. The configurable timeout addition is non-breaking. No data-correctness or connection-state bugs were introduced.

No files require special attention; the two suggestions are purely cosmetic/diagnostic improvements.

Important Files Changed

Filename Overview
src/Client.php Refactors receive() to fail fast on recv()===false, adds configurable $timeout constructor parameter, and resets the idle deadline on each real chunk to allow large multi-chunk responses to complete.

Reviews (3): Last reviewed commit: "refactor: rename receiveTimeout property..." | Re-trigger Greptile

Comment thread src/Client.php Outdated

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/Client.php`:
- Around line 464-487: Update the timeout exceptions in the receive handling
logic of Client to pass MongoDB’s standard socket timeout code 11601 as the
constructor’s code argument. Apply this to the deadline check and both
false/empty recv timeout branches, preserving their existing messages and
conditions.
- Around line 434-438: Update send() to use the original non-zero error code
when the retry result is zero by applying a falsey fallback instead of
null-coalescing. Ensure reconnect send failures use error code 9001 and
receive() timeout exceptions use 11601, preserving correct isNetworkError() and
isTimeoutError() classification.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 37258707-e024-4a48-ba60-12b9c18f4bb9

📥 Commits

Reviewing files that changed from the base of the PR and between 44eeb4d and bd1a4cc.

📒 Files selected for processing (1)
  • src/Client.php

Comment thread src/Client.php Outdated
Comment thread src/Client.php
ChiragAgg5k and others added 2 commits July 14, 2026 20:39
Reset the receive deadline on each chunk so large transfers are not cut
off mid-stream, expose setTimeout/getTimeout, and use Mongo socket codes
so isTimeoutError()/isNetworkError() classify these failures.

Co-authored-by: Cursor <cursoragent@cursor.com>
Replace setTimeout/getTimeout with an optional trailing $timeout
constructor argument (default 30s) so callers configure it at connect time.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@loks0n loks0n merged commit f6ef5f1 into main Jul 14, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants