Skip to content

Suppress recv error#34

Merged
abnegate merged 1 commit intomainfrom
fix-recv-error
Mar 18, 2026
Merged

Suppress recv error#34
abnegate merged 1 commit intomainfrom
fix-recv-error

Conversation

@abnegate
Copy link
Member

@abnegate abnegate commented Mar 14, 2026

Summary by CodeRabbit

  • Refactor
    • Improved error handling to suppress non-critical warnings in communication operations. This change reduces log noise while preserving all existing functionality, timeout mechanisms, and system behavior.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 14, 2026

📝 Walkthrough

Walkthrough

The receive() method in src/Client.php now suppresses warnings and errors from the underlying recv() call using the error suppression operator (@). The surrounding logic for backoff, length checks, and parsing remains unchanged.

Changes

Cohort / File(s) Summary
Error Suppression
src/Client.php
Added @ error suppression operator to recv() call in the receive() method to silence warnings/errors from the underlying client operation.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A whisper here, a shush there,
Warnings fade into thin air,
The recv() call now silent, sleek,
No errors peep, no alarms squeak!
In CodeRabbit's caring paws so bright,
We code the night away, just right. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly describes the main change: adding error suppression to the recv() method call using the @ operator.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-recv-error
📝 Coding Plan
  • Generate coding plan for human review comments

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 and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/Client.php (1)

409-426: ⚠️ Potential issue | 🟡 Minor

Error suppression may hide important diagnostic information.

Using @ to suppress errors from recv() silently hides connection issues, timeouts, and socket errors that could be valuable for debugging. While the code handles false returns, you lose visibility into why the call failed.

If this change is addressing a specific noisy warning in production, could you clarify what error is being suppressed? That context would help determine if this is the right approach or if the root cause should be addressed instead.

Alternatively, you could check $this->client->errCode after a failed recv() to capture and log the specific error before continuing (using swoole_strerror($code) to convert the errno to a readable message).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Client.php` around lines 409 - 426, Remove the error suppression operator
on $this->client->recv() and, when recv() returns false or empty, read
$this->client->errCode (and convert with swoole_strerror when available) and log
or attach that error info before continuing/throwing; adjust the block around
the recv() call in the Client class (the do/while loop handling $chunk,
$attempts, $maxAttempts, $sleepTime and CoroutineClient/Coroutine::sleep()) so
diagnostic details from the failed recv are captured (and only suppress or
silence specific known benign warnings after documenting why).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/Client.php`:
- Around line 409-426: Remove the error suppression operator on
$this->client->recv() and, when recv() returns false or empty, read
$this->client->errCode (and convert with swoole_strerror when available) and log
or attach that error info before continuing/throwing; adjust the block around
the recv() call in the Client class (the do/while loop handling $chunk,
$attempts, $maxAttempts, $sleepTime and CoroutineClient/Coroutine::sleep()) so
diagnostic details from the failed recv are captured (and only suppress or
silence specific known benign warnings after documenting why).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 24a8f7eb-84a3-4b4a-8d17-57abdb0b02f2

📥 Commits

Reviewing files that changed from the base of the PR and between 83dbcde and 0bc8b4a.

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

@abnegate abnegate merged commit 677a21c into main Mar 18, 2026
4 checks passed
@abnegate abnegate deleted the fix-recv-error branch March 18, 2026 02:45
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.

1 participant