Treat remote 404 as missing preview in readRemoteFile#197
Conversation
Navigating Pear Issues logged a RelayFileApiError 404 from the
integrations:read-remote-file IPC handler. The Issues store enriches
each Linear issue with its linked GitHub record by reading a synthetic
path (/github/repos/{owner}/{repo}/issues/{number}.json) derived from
Linear sync metadata. Because historical provider records are not
downloaded locally, that file often does not exist remotely and the
RelayFile API returns 404.
The renderer already recovers (readGithubLink wraps the read in
.catch(() => null) and falls back to a basic link), but readRemoteFile
threw on the 404, so Electron logged the rejected IPC handler regardless.
Make readRemoteFile catch a 404 and return a { kind: 'missing' } preview,
mirroring how the local filesystem path (readTextPreview) already reports
absent files. Both callers act only on kind === 'text', so they degrade
cleanly: missing GitHub enrichment falls back to the basic link, and a
race where an issue file vanishes between listing and read skips that
issue rather than failing the board.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
📝 WalkthroughWalkthrough
ChangesRemote file 404 error handling
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. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
pr-reviewer could not complete review for #197 in AgentWorkforce/pear. |
1 similar comment
|
pr-reviewer could not complete review for #197 in AgentWorkforce/pear. |
![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 `@src/main/integrations.test.ts`:
- Around line 956-973: Add a test that ensures non-404 errors from the relay are
re-thrown: copy the pattern from the existing 404 test in integrations.test.ts
but have mock.relayClient.readFile.mockImplementationOnce throw an Error/object
with status: 500 (or a network error), then call new
IntegrationsManager().readRemoteFile('project-1', '/slack/...json') and assert
the promise rejects with that same error (use expect(...).rejects.toThrow or
.rejects.toMatchObject to verify the error/status). Reference readRemoteFile,
IntegrationsManager and mock.relayClient.readFile when locating where to insert
the test.
🪄 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 Plus
Run ID: 0701d3e3-ca9e-4720-81e8-6c66499d039e
📒 Files selected for processing (2)
src/main/integrations.test.tssrc/main/integrations.ts
| it('returns a missing preview when an in-scope remote read 404s instead of rejecting', async () => { | ||
| // Historical provider records (e.g. the GitHub issue JSON synthesized from | ||
| // Linear sync metadata) are not downloaded locally, so an in-scope read can | ||
| // legitimately 404. It must degrade to a missing preview rather than reject | ||
| // the IPC handler. | ||
| mock.relayClient.readFile.mockImplementationOnce(async (_workspaceId: string, path: string) => { | ||
| mock.readFileCalls.push({ workspaceId: _workspaceId, path }) | ||
| throw Object.assign(new Error('not found'), { status: 404, code: 'not_found' }) | ||
| }) | ||
|
|
||
| const manager = new IntegrationsManager() | ||
| const preview = await manager.readRemoteFile( | ||
| 'project-1', | ||
| '/slack/channels/C123/messages/1713220123_001100.json' | ||
| ) | ||
|
|
||
| expect(preview).toMatchObject({ kind: 'missing', size: 0 }) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add test coverage for non-404 errors being re-thrown.
The new 404 test correctly verifies graceful degradation, but there's no test ensuring that non-404 errors (e.g., 500, network failures) are still thrown as expected (per line 1082 in integrations.ts). Without this coverage, a bug that swallows all errors could go undetected.
As per coding guidelines, integration notification changes should include regression tests beyond the happy path. Consider adding a test case that mocks relayClient.readFile to throw a non-404 error (e.g., { status: 500 }) and asserts that readRemoteFile rejects with that error.
🧪 Proposed test for non-404 error propagation
})
+ it('re-throws non-404 remote read errors instead of returning a missing preview', async () => {
+ mock.relayClient.readFile.mockImplementationOnce(async (_workspaceId: string, path: string) => {
+ mock.readFileCalls.push({ workspaceId: _workspaceId, path })
+ throw Object.assign(new Error('internal server error'), { status: 500 })
+ })
+
+ const manager = new IntegrationsManager()
+
+ await expect(
+ manager.readRemoteFile('project-1', '/slack/channels/C123/messages/1713220123_001100.json')
+ ).rejects.toThrow('internal server error')
+ })
+
it('rejects targeted remote file reads outside the project integration scope', async () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('returns a missing preview when an in-scope remote read 404s instead of rejecting', async () => { | |
| // Historical provider records (e.g. the GitHub issue JSON synthesized from | |
| // Linear sync metadata) are not downloaded locally, so an in-scope read can | |
| // legitimately 404. It must degrade to a missing preview rather than reject | |
| // the IPC handler. | |
| mock.relayClient.readFile.mockImplementationOnce(async (_workspaceId: string, path: string) => { | |
| mock.readFileCalls.push({ workspaceId: _workspaceId, path }) | |
| throw Object.assign(new Error('not found'), { status: 404, code: 'not_found' }) | |
| }) | |
| const manager = new IntegrationsManager() | |
| const preview = await manager.readRemoteFile( | |
| 'project-1', | |
| '/slack/channels/C123/messages/1713220123_001100.json' | |
| ) | |
| expect(preview).toMatchObject({ kind: 'missing', size: 0 }) | |
| }) | |
| it('returns a missing preview when an in-scope remote read 404s instead of rejecting', async () => { | |
| // Historical provider records (e.g. the GitHub issue JSON synthesized from | |
| // Linear sync metadata) are not downloaded locally, so an in-scope read can | |
| // legitimately 404. It must degrade to a missing preview rather than reject | |
| // the IPC handler. | |
| mock.relayClient.readFile.mockImplementationOnce(async (_workspaceId: string, path: string) => { | |
| mock.readFileCalls.push({ workspaceId: _workspaceId, path }) | |
| throw Object.assign(new Error('not found'), { status: 404, code: 'not_found' }) | |
| }) | |
| const manager = new IntegrationsManager() | |
| const preview = await manager.readRemoteFile( | |
| 'project-1', | |
| '/slack/channels/C123/messages/1713220123_001100.json' | |
| ) | |
| expect(preview).toMatchObject({ kind: 'missing', size: 0 }) | |
| }) | |
| it('re-throws non-404 remote read errors instead of returning a missing preview', async () => { | |
| mock.relayClient.readFile.mockImplementationOnce(async (_workspaceId: string, path: string) => { | |
| mock.readFileCalls.push({ workspaceId: _workspaceId, path }) | |
| throw Object.assign(new Error('internal server error'), { status: 500 }) | |
| }) | |
| const manager = new IntegrationsManager() | |
| await expect( | |
| manager.readRemoteFile('project-1', '/slack/channels/C123/messages/1713220123_001100.json') | |
| ).rejects.toThrow('internal server error') | |
| }) |
🤖 Prompt for 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.
In `@src/main/integrations.test.ts` around lines 956 - 973, Add a test that
ensures non-404 errors from the relay are re-thrown: copy the pattern from the
existing 404 test in integrations.test.ts but have
mock.relayClient.readFile.mockImplementationOnce throw an Error/object with
status: 500 (or a network error), then call new
IntegrationsManager().readRemoteFile('project-1', '/slack/...json') and assert
the promise rejects with that same error (use expect(...).rejects.toThrow or
.rejects.toMatchObject to verify the error/status). Reference readRemoteFile,
IntegrationsManager and mock.relayClient.readFile when locating where to insert
the test.
Source: Coding guidelines
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the IntegrationsManager to gracefully handle 404 errors when reading remote files by returning a missing preview object instead of throwing an error, mirroring local filesystem behavior. A corresponding unit test has been added to verify this behavior. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
pr-reviewer could not complete review for #197 in AgentWorkforce/pear. |
1 similar comment
|
pr-reviewer could not complete review for #197 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #197 in AgentWorkforce/pear. |
1 similar comment
|
pr-reviewer could not complete review for #197 in AgentWorkforce/pear. |
1 participant
Problem
Navigating Pear Issues logged an error in the main process:
Root cause
The Issues store enriches each Linear issue with its linked GitHub record by reading a synthetic path
/github/repos/{owner}/{repo}/issues/{number}.json(issues-store.ts:192), derived from Linear sync metadata. This project's integration setup does not download historical provider records locally, so that file often doesn't exist remotely → the RelayFile API returns 404.The renderer already recovers —
readGithubLinkwraps the read in.catch(() => null)and falls back to a basic GitHub link — so the Issues view itself wasn't broken. ButreadRemoteFilethrew on the 404, so Electron logged the rejected IPC handler regardless of the renderer's catch.Fix
readRemoteFilenow catches a 404 and returns a{ kind: 'missing' }preview instead of throwing, mirroring how the local filesystem path (readTextPreviewinfilesystem.ts:60) already reports absent files.Both callers act only on
kind === 'text', so they degrade cleanly:The scope-guard rejections (out-of-scope reads) are unaffected — they throw before the remote call.
Testing
missingpreview rather than rejecting.integrations.test.tspass.🤖 Generated with Claude Code