Conversation
|
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ 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 |
Add comprehensive test coverage for error handling in node.reducer for: - NoValueCollector update rejection - Undefined value detection - Type validation for SingleValueCollector (must be string) - MultiValueCollector object rejection - PollingCollector update rejection All tests fail as expected in RED phase (throws are unhandled by reducer).
…e writes Eliminates thrown errors from updateCollectorValues — all validation failures now write to collector.error instead, keeping Redux state predictable and observable rather than blowing up the call stack. Also adds an explicit PollingCollector type guard before the category branches so that polling collectors (which are categorized as SingleValueAutoCollector) are correctly rejected as read-only.
…update Stale validation errors persisted in state after a user corrected their input — a subsequent valid dispatch left the previous error message visible. Mirror the pattern already used in pollCollectorValues by resetting collector.error to null on every successful value-assignment path in updateCollectorValues.
ryanbas21
force-pushed
the
remove-throw-reducers
branch
from
fe20632 to
1e2483b
Compare
|
View your CI Pipeline Execution ↗ for commit 1e2483b
☁️ Nx Cloud last updated this comment at |
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud is proposing a fix for your failed CI:
We updated the stale test "should throw with no collectors" to align with the PR's intent of replacing all reducer throws with state-based error writes. The test now verifies that an UnknownCollector is pushed into state with error: 'No collector found to update' instead of asserting the reducer throws, which it no longer does. This fix unblocks the failing @forgerock/davinci-client:test task.
Tip
✅ We verified this fix by re-running @forgerock/davinci-client:test.
diff --git a/packages/davinci-client/src/lib/node.reducer.test.ts b/packages/davinci-client/src/lib/node.reducer.test.ts
index 51ac6f5..44d5159 100644
--- a/packages/davinci-client/src/lib/node.reducer.test.ts
+++ b/packages/davinci-client/src/lib/node.reducer.test.ts
@@ -344,7 +344,7 @@ describe('The node collector reducer', () => {
]);
});
- it('should throw with no collectors', () => {
+ it('should set error on UnknownCollector when no collector is found', () => {
const action = {
type: 'node/update',
payload: {
@@ -372,7 +372,9 @@ describe('The node collector reducer', () => {
},
},
];
- expect(() => nodeCollectorReducer(state, action)).toThrowError('No collector found to update');
+ const result = nodeCollectorReducer(state, action);
+ const unknownCollector = result.find((c) => c.id === 'submit-1');
+ expect(unknownCollector?.error).toBe('No collector found to update');
});
it('should set error on ActionCollector when update is attempted', () => {
Or Apply changes locally with:
npx nx-cloud apply-locally AcI8-rIPR
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
ryanbas21
force-pushed
the
remove-throw-reducers
branch
from
1e2483b to
1781cf3
Compare
@forgerock/davinci-client
@forgerock/device-client
@forgerock/journey-client
@forgerock/oidc-client
@forgerock/protect
@forgerock/sdk-types
@forgerock/sdk-utilities
@forgerock/iframe-manager
@forgerock/sdk-logger
@forgerock/sdk-oidc
@forgerock/sdk-request-middleware
@forgerock/storage
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #626 +/- ##
===========================================
- Coverage 70.90% 55.92% -14.98%
===========================================
Files 53 22 -31
Lines 2021 2489 +468
Branches 377 358 -19
===========================================
- Hits 1433 1392 -41
- Misses 588 1097 +509
🚀 New features to boost your workflow:
|
|
Deployed 291fa9a to https://ForgeRock.github.io/ping-javascript-sdk/pr-626/291fa9adcf2e2cd2fc28f4e0fc44d78ba7b8bd8f branch gh-pages in ForgeRock/ping-javascript-sdk |
📦 Bundle Size Analysis📦 Bundle Size Analysis🚨 Significant Changes🔻 @forgerock/device-client - 0.0 KB (-10.0 KB, -100.0%) 📊 Minor Changes📈 @forgerock/davinci-client - 48.9 KB (+0.0 KB) ➖ No Changes➖ @forgerock/device-client - 10.0 KB 14 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
2 participants
JIRA Ticket
N/A
Description
Remove throws from reducers, since they are impure. This is tricky because we have to determine what we want in state then. I opted for an unknown collector but im not positive of this its weird.