fix(config): log RPC URL and core mode as strings, not object wrappers#2459
Conversation
storeRpcUrl logged { url: '...' } and storeCoreMode logged { mode }
causing "[object Object]" in diagnostic output and hiding the actual
value in user-reported logs. Log the string values directly.
Adds a regression test asserting no object argument appears in the
debug log call for storeRpcUrl.
Refs tinyhumansai#2437 (sub-issue B)
|
Warning Review limit reached
Your plan currently allows 4 reviews/hour. Refill in 7 minutes and 37 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, 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 trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated storeCoreMode to log the primitive ChangesDebug Logging Simplification
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Clean, focused fix — nice work. Both storeRpcUrl and storeCoreMode changes are correct; the object-wrapper { url: url.trim() } / { mode } was indeed printing [object Object] in non-DevTools contexts (CEF logs, production consoles). Test covers the storeRpcUrl path well.
[minor] The storeCoreMode fix (line 258) is identical in nature but has no corresponding test. Not blocking since it's a trivial one-line change and the pattern is proven by the existing test, but worth adding for completeness if you're doing a follow-up.
| File | Change |
|---|---|
configPersistence.ts:88 |
{ url: url.trim() } → url.trim() |
configPersistence.ts:258 |
{ mode } → mode |
configPersistence.test.ts |
+1 regression test for object-wrapper logging |
LGTM — moving to approved queue.
…ycyrus review) Add regression test verifying storeCoreMode logs the mode string directly rather than an object wrapper, mirroring the existing storeRpcUrl test. Requested in reviewer comment: "not blocking since it's a trivial one-line change and the pattern is proven by the existing test, but worth adding."
|
Thanks @graycyrus! Added the |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@app/src/utils/__tests__/configPersistence.test.ts`:
- Around line 433-443: The test "logs the mode string directly, not an object
wrapper" currently spies on console.debug but only calls spy.mockRestore() at
the end, which can leak the spy if an assertion throws; update this test to wrap
the assertions and any call to spy.mock.calls in a try/finally block so that
spy.mockRestore() is always executed. Specifically, inside the test that calls
storeCoreMode('cloud') and inspects spy.mock.calls, move the assertions into a
try block and put spy.mockRestore() in the finally block to guarantee cleanup
even on failure.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2d76c217-bd05-4c56-a481-2123e4adcee4
📒 Files selected for processing (1)
app/src/utils/__tests__/configPersistence.test.ts
…bit) Ensure mockRestore() always runs even when an assertion throws, preventing console.debug spy from leaking into subsequent tests. Applied to both the storeRpcUrl and storeCoreMode log-format tests.
# Conflicts: # app/src/utils/configPersistence.ts
…erge The merge with main re-introduced the duplicate keys that tinyhumansai#2495 had already removed, because the PR branch and main both held copies in different chunk positions. Drop the second occurrence to restore Type Check green.
3 participants
Summary
storeRpcUrl: changeconsole.debug('[configPersistence] Stored RPC URL:', { url: url.trim() })→ logurl.trim()directly so the value appears as a readable string rather than[object Object].storeCoreMode: same fix —{ mode }→mode.storeRpcUrldebug call.Problem
configPersistence.tslogged wrapper objects{ url: '...' }and{ mode }in its debug lines. In browser DevTools (and in CEF renderer logs) these print as[object Object], actively hiding the stored value from diagnostic output. This was flagged as sub-issue B in #2437 as the cause of masked root-cause investigation: the log lineStored RPC URL: [object Object]hid the fact that no cloud RPC URL had been stored, making the remote-core connection failure harder to triage.Solution
Log the scalar values directly. One-line fix per site; no behaviour change.
Submission Checklist
'logs the URL string directly, not an object wrapper'inconfigPersistence.test.tsRefs #2437(sub-issue B)Impact
Related
[configPersistence] Stored RPC URL: [object Object]printer bug)AI Authored PR Metadata
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check— cleanpnpm --filter openhuman-app compile— 0 errorspnpm --filter openhuman-app lint— 0 new errorspnpm debug unit configPersistence— 68 tests pass (1 new)Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
localStorageread/write semantics unchanged; only theconsole.debugargument format changedDuplicate / Superseded PR Handling
Summary by CodeRabbit
Tests
Refactor