Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 30 minutes and 42 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (7)
WalkthroughAdapterClient error handling updated to treat Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ProtocolAdapter/adapterclient.cpp`:
- Around line 212-220: The JSON-RPC error handling must ignore stale errors for
overlapping validateDataPoint calls: in the error branch for method ==
"adapter.validateDataPoint" (in adapterclient.cpp) only treat the error as a
validation result when _pendingAuxRequests.value(method, -1) == id (and _state
is AWAITING_CONFIG or ACTIVE); if the stored request id does not match the
incoming id, simply drop/return so the stale error does not fall through to the
session-fatal path. Ensure you remove the pending entry and emit
validateDataPointResult(false, errorMsg) only when the id matches; otherwise do
nothing (mirroring how handleLifecycleResponse ignores late replies).
In `@tests/integration/tst_dummyadapter.cpp`:
- Around line 258-273: The test
TestDummyAdapter::validateDataPointReturnsFalseForInvalidExpression must assert
the session remains non-fatal: after waiting for spyResult, add an assertion
that spyError.count() == 0 to ensure no sessionError was emitted, then send an
additional auxiliary request (e.g., call _pClient->describeAdapter or another
_pClient->validateDataPoint with a valid expression) and wait for its
corresponding QSignalSpy to verify the client stays in AWAITING_CONFIG and
responds normally; reference spies spyError and spyResult and the _pClient
methods to locate where to add these post-response checks.
🪄 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: 72a03234-26ea-4027-94c8-bfc17121ed08
⛔ Files ignored due to path filters (2)
adapters/dummymodbusadapter.exeis excluded by!**/*.exeadapters/modbusadapter.exeis excluded by!**/*.exe
📒 Files selected for processing (5)
adapters/dummymodbusadapteradapters/modbusadaptersrc/ProtocolAdapter/adapterclient.cpptests/integration/tst_dummyadapter.cpptests/integration/tst_dummyadapter.h
| /* For auxiliary requests, a JSON-RPC error is a non-fatal validation result rather | ||
| than a session-level failure. Translate to the corresponding result signal. */ | ||
| if (method == QStringLiteral("adapter.validateDataPoint") && | ||
| (_state == State::AWAITING_CONFIG || _state == State::ACTIVE) && _pendingAuxRequests.value(method, -1) == id) | ||
| { | ||
| _pendingAuxRequests.remove(method); | ||
| emit validateDataPointResult(false, errorMsg); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Ignore stale adapter.validateDataPoint errors instead of escalating them.
If validateDataPoint() is called again before the first reply arrives, the second call overwrites _pendingAuxRequests["adapter.validateDataPoint"]. A late JSON-RPC error from the older request then misses this branch and falls through to the session-fatal path, even though late success responses are already ignored in handleLifecycleResponse(). That makes overlapping validations capable of dropping an otherwise healthy session.
Suggested fix
- if (method == QStringLiteral("adapter.validateDataPoint") &&
- (_state == State::AWAITING_CONFIG || _state == State::ACTIVE) && _pendingAuxRequests.value(method, -1) == id)
+ if (method == QStringLiteral("adapter.validateDataPoint") &&
+ (_state == State::AWAITING_CONFIG || _state == State::ACTIVE))
{
+ if (_pendingAuxRequests.value(method, -1) != id)
+ {
+ qCWarning(scopeComm) << "AdapterClient: ignoring stale error for" << method;
+ return;
+ }
_pendingAuxRequests.remove(method);
emit validateDataPointResult(false, errorMsg);
return;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ProtocolAdapter/adapterclient.cpp` around lines 212 - 220, The JSON-RPC
error handling must ignore stale errors for overlapping validateDataPoint calls:
in the error branch for method == "adapter.validateDataPoint" (in
adapterclient.cpp) only treat the error as a validation result when
_pendingAuxRequests.value(method, -1) == id (and _state is AWAITING_CONFIG or
ACTIVE); if the stored request id does not match the incoming id, simply
drop/return so the stale error does not fall through to the session-fatal path.
Ensure you remove the pending entry and emit validateDataPointResult(false,
errorMsg) only when the id matches; otherwise do nothing (mirroring how
handleLifecycleResponse ignores late replies).
Split the ID-match guard in onErrorReceived so a validateDataPoint error in the correct state always returns early: emit validateDataPointResult only when the pending request ID matches, otherwise silently drop the stale error rather than falling through to the session-fatal path. Extend the validateDataPointReturnsFalseForInvalidExpression test to assert spyError stays empty and that a follow-up auxiliary request succeeds, confirming the client remains operational after a validation failure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests