fix(composio): gracefully handle unknown method for trigger settings RPC#1784
Conversation
When users have a stale core sidecar that predates the composio trigger settings controller, the frontend now returns safe defaults instead of propagating "unknown method" errors to Sentry. Closes tinyhumansai#1597
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughWrap Composio trigger RPCs in try/catch; on core "unknown method" errors return safe fallbacks (empty ConfigSnapshot for update; default trigger settings for get). Add Vitest tests verifying fallbacks and that other errors are rethrown. ChangesComposio Trigger Settings Error Resilience
Sequence DiagramsequenceDiagram
participant Frontend as Frontend/Tauri
participant CoreRpc as callCoreRpc
participant ErrorCheck as tauriErrorMessage
Frontend->>CoreRpc: openhuman.config_get_composio_trigger_settings / update
CoreRpc-->>Frontend: reject (error)
Frontend->>ErrorCheck: tauriErrorMessage(error)
alt message == "unknown method"
Frontend-->>Frontend: return fallback (default settings or empty ConfigSnapshot + logs: [])
else
Frontend-->>Frontend: rethrow error
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/tauriCommands/config.ts`:
- Around line 307-310: The stale-core fallback is returning an unsafe cast "{}
as ConfigSnapshot" which can break callers that expect required fields; replace
that cast with a real default ConfigSnapshot object containing all required
properties (at minimum provide safe defaults for config, workspace_dir, and
config_path) in the branch where tauriErrorMessage(err).includes('unknown
method') so consumers reading those fields won't crash (update the return inside
that if block that currently returns { result: {} as ConfigSnapshot, logs: []
}).
🪄 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: 06a57d0d-56ee-4872-a065-b809004367e5
📒 Files selected for processing (2)
app/src/utils/tauriCommands/config.test.tsapp/src/utils/tauriCommands/config.ts
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Walkthrough
Clean, focused fix for #1597 — wraps the two composio trigger settings RPC calls in try/catch to gracefully degrade when users are running a stale core sidecar that doesn't have these methods registered. The get function returns safe defaults, the update returns a no-op response. Good test coverage with 4 new tests. One type-safety issue to address and a logging gap per project standards.
Change Summary
| File | Change type | Description |
|---|---|---|
app/src/utils/tauriCommands/config.ts |
Modified | Added try/catch with "unknown method" detection around both composio RPC calls, returning fallback values |
app/src/utils/tauriCommands/config.test.ts |
Modified | Added 4 tests: graceful degradation + error re-throw for both get and update functions |
Per-file analysis
config.ts
The openhumanGetComposioTriggerSettings fallback is well-constructed — { triage_disabled: false, triage_disabled_toolkits: [] } matches the ComposioTriggerSettings interface properly.
The openhumanUpdateComposioTriggerSettings fallback uses {} as ConfigSnapshot which is an unsafe cast — see inline comment.
Neither catch block logs the graceful degradation, which makes silent failures harder to diagnose in the field.
config.test.ts
Tests are solid — they cover both the graceful path and the re-throw path for each function. Issue reference (#1597) in test names is a nice touch for traceability. Structure matches existing test patterns in the file.
| } catch (err) { | ||
| if (tauriErrorMessage(err).includes('unknown method')) { | ||
| // Stale core sidecar predates composio trigger settings (#1597). | ||
| return { result: {} as ConfigSnapshot, logs: [] }; |
There was a problem hiding this comment.
[major] {} as ConfigSnapshot is an unsafe type cast that hides missing required fields (config, workspace_dir, config_path). If any future caller reads these fields from the update response, they'll get undefined at runtime while TypeScript thinks they're populated.
The current caller (ComposioTriagePanel) doesn't inspect the result beyond awaiting it, so this won't break today — but it's a footgun.
Suggestion — provide a structurally valid default:
return { result: { config: {}, workspace_dir: '', config_path: '' } as ConfigSnapshot, logs: [] };Or, if you'd rather be explicit that this is a degraded response, consider a Partial<ConfigSnapshot> return type for the fallback path (though that's a larger change).
There was a problem hiding this comment.
Fixed in 9e92ff0 — replaced the unsafe cast with a structurally valid default: { config: {}, workspace_dir: '', config_path: '' }.
| }); | ||
| } catch (err) { | ||
| if (tauriErrorMessage(err).includes('unknown method')) { | ||
| // Stale core sidecar predates composio trigger settings (#1597). |
There was a problem hiding this comment.
[minor] Per project standards (known issues watchlist), new/changed error flows should include debug logging with grep-friendly prefixes. These catch blocks silently degrade, which makes it harder to diagnose version-mismatch issues in the field.
Suggestion — add a debug log in both catch blocks:
import debug from 'debug';
const log = debug('composio:rpc');
// ...
log('graceful degradation: stale core lacks openhuman.config_update_composio_trigger_settings (#1597)');This way DEBUG=composio:* surfaces these in dev, and they're zero-cost in production.
There was a problem hiding this comment.
Fixed in 9e92ff0 — added debug('composio:rpc') logging in both catch blocks. Surfaces via DEBUG=composio:* in dev, zero-cost in production.
- Replace `{} as ConfigSnapshot` with structurally valid default
`{ config: {}, workspace_dir: '', config_path: '' }`
- Add debug logging in both catch blocks for field diagnostics
|
@graycyrus CI is failing on changes in this PR — please fix before review. |
Wrap long debug() log lines and inline the small toEqual object so `pnpm format:check` passes in CI.
2 participants
Summary
openhumanGetComposioTriggerSettingsandopenhumanUpdateComposioTriggerSettingsin try/catch that detects "unknown method" errors from stale core sidecar binariestriage_disabled: false, empty toolkits array) instead of propagating the error to SentryCloses #1597
Test plan
Summary by CodeRabbit