FlowyLabs/llink
1
0
Fork 0
Code Issues 71 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix: add platform abstraction for deeplink #251

Merged
talksik merged 3 commits from fix-web-deep-link into main 2026-06-09 17:01:23 +00:00
Conversation 7 Commits 3 Files Changed 5
+28 -8
5 changed files with 28 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
js/desktop/src/App.tsx
+10 -4
View File
@@ -56,10 +56,16 @@ function DeepLinkNavigationListener() {
const navigate = useNavigate();
coderabbitai[bot] commented 2026-06-09 16:37:30 +00:00 (Migrated from github.com)
Review
Copy Link
Copy Source

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add error handling for getPending() promise.

The getPending() call has no error handler. If the underlying Electron IPC or platform implementation throws or rejects, this will result in an unhandled promise rejection that could crash the app or surface console errors.

🛡️ Proposed fix to add error handling 
-    platform.deepLink.getPending().then((path) => {
-      if (path) navigate(path);
-    });
+    platform.deepLink
+      .getPending()
+      .then((path) => {
+        if (path) navigate(path);
+      })
+      .catch((error) => {
+        console.error('Failed to retrieve pending deep link:', error);
+      });
📝 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.

    platform.deepLink
      .getPending()
      .then((path) => {
        if (path) navigate(path);
      })
      .catch((error) => {
        console.error('Failed to retrieve pending deep link:', 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 `@js/desktop/src/App.tsx` around lines 59 - 61, The call to
platform.deepLink.getPending() lacks rejection handling; wrap the promise usage
so errors are caught and logged or handled (e.g., add a .catch handler or await
inside a try/catch) around platform.deepLink.getPending() and ensure failures do
not block navigation; specifically update the block that calls
platform.deepLink.getPending() (the call site referencing
platform.deepLink.getPending and navigate) to catch errors and call an
appropriate error handler or logger instead of leaving the promise unhandled.

✅ Addressed in commit 5275f38

_⚠️ Potential issue_ | _🟠 Major_ | _⚡ Quick win_ **Add error handling for `getPending()` promise.** The `getPending()` call has no error handler. If the underlying Electron IPC or platform implementation throws or rejects, this will result in an unhandled promise rejection that could crash the app or surface console errors. <details> <summary>🛡️ Proposed fix to add error handling</summary> ```diff - platform.deepLink.getPending().then((path) => { - if (path) navigate(path); - }); + platform.deepLink + .getPending() + .then((path) => { + if (path) navigate(path); + }) + .catch((error) => { + console.error('Failed to retrieve pending deep link:', error); + }); ``` </details> <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **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. ```suggestion platform.deepLink .getPending() .then((path) => { if (path) navigate(path); }) .catch((error) => { console.error('Failed to retrieve pending deep link:', error); }); ``` </details> <!-- suggestion_end --> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@js/desktop/src/App.tsx` around lines 59 - 61, The call to platform.deepLink.getPending() lacks rejection handling; wrap the promise usage so errors are caught and logged or handled (e.g., add a .catch handler or await inside a try/catch) around platform.deepLink.getPending() and ensure failures do not block navigation; specifically update the block that calls platform.deepLink.getPending() (the call site referencing platform.deepLink.getPending and navigate) to catch errors and call an appropriate error handler or logger instead of leaving the promise unhandled. ``` </details> <!-- fingerprinting:phantom:poseidon:puma --> <!-- cr-comment:v1:d14afe494197cfa7951aeb49 --> <!-- This is an auto-generated reply by CodeRabbit --> ✅ Addressed in commit 5275f38 <!-- This is an auto-generated comment by CodeRabbit -->
talksik commented 2026-06-09 16:47:54 +00:00 (Migrated from github.com)
Review
Copy Link
Copy Source

Shouldn't your fix use either reportError or logError?

Shouldn't your fix use either reportError or logError?
coderabbitai[bot] commented 2026-06-09 16:48:41 +00:00 (Migrated from github.com)
Review
Copy Link
Copy Source
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for reportError and logError definitions and usages
rg -n "reportError|logError" --type ts --type tsx -C2

Length of output: 397

🏁 Script executed:

#!/bin/bash
# Search for reportError and logError definitions and usages
rg -n "reportError|logError" -C2

Length of output: 28070

@talksik — you're right. The codebase has a clear distinction in js/desktop/src/lib/errors.ts:

  • logError — "Expected-but-recordable failures. Breadcrumb only — never pages anyone." (dev-only console output)
  • reportError — "Unexpected failures the user may not see. Always captured." (always sent to the observability sink)

For getPending(), a rejection is recoverable (navigation simply doesn't happen) and low-impact, which fits logError's contract. This also matches the pattern used for similar call-site errors like autoplay.beep and soundEffects.preload. Updated suggestion:

+import { logError } from '`@/lib/errors`';

     platform.deepLink
       .getPending()
       .then((path) => {
         if (path) navigate(path);
       })
-      .catch((error) => {
-        console.error('Failed to retrieve pending deep link:', error);
-      });
+      .catch((err) => {
+        logError(err, { scope: 'deepLink.getPending' });
+      });

That said, if the team treats any IPC rejection as unexpected (similar to how ipc-utils.ts uses reportError for IPC handler errors on the main-process side), reportError would also be justified. The call is yours based on the severity convention you want to apply here.

<details> <summary>🧩 Analysis chain</summary> 🏁 Script executed: ```shell #!/bin/bash # Search for reportError and logError definitions and usages rg -n "reportError|logError" --type ts --type tsx -C2 ``` Length of output: 397 --- 🏁 Script executed: ```shell #!/bin/bash # Search for reportError and logError definitions and usages rg -n "reportError|logError" -C2 ``` Length of output: 28070 --- </details> --- `@talksik` — you're right. The codebase has a clear distinction in `js/desktop/src/lib/errors.ts`: - **`logError`** — _"Expected-but-recordable failures. Breadcrumb only — never pages anyone."_ (dev-only console output) - **`reportError`** — _"Unexpected failures the user may not see. Always captured."_ (always sent to the observability sink) For `getPending()`, a rejection is recoverable (navigation simply doesn't happen) and low-impact, which fits `logError`'s contract. This also matches the pattern used for similar call-site errors like `autoplay.beep` and `soundEffects.preload`. Updated suggestion: ```diff +import { logError } from '`@/lib/errors`'; platform.deepLink .getPending() .then((path) => { if (path) navigate(path); }) - .catch((error) => { - console.error('Failed to retrieve pending deep link:', error); - }); + .catch((err) => { + logError(err, { scope: 'deepLink.getPending' }); + }); ``` That said, if the team treats any IPC rejection as unexpected (similar to how `ipc-utils.ts` uses `reportError` for IPC handler errors on the main-process side), `reportError` would also be justified. The call is yours based on the severity convention you want to apply here. <!-- This is an auto-generated reply by CodeRabbit -->
talksik commented 2026-06-09 16:49:52 +00:00 (Migrated from github.com)
Review
Copy Link
Copy Source

@coderabbitai help

@coderabbitai help
useEffect(() => {
window.electronDeepLink.getPending().then((path) => {
if (path) navigate(path);
});
return window.electronDeepLink.onNavigate((path) => navigate(path));
platform.deepLink
.getPending()
.then((path) => {
if (path) navigate(path);
})
.catch((err) => {
logError(err, { scope: 'deepLink.getPending' });
});
return platform.deepLink.onNavigate((path) => navigate(path));
}, [navigate]);
return null;
js/desktop/src/features/network-root.tsx
+1 -4
View File
@@ -30,8 +30,7 @@ export default function NetworkRoot() {
setSearchParams(
(prev) => {
const params = new URLSearchParams(prev);
if (next === 'open') params.delete('status');
else params.set('status', next);
params.set('status', next);
return params;
},
{ replace: true },
@@ -56,7 +55,6 @@ export default function NetworkRoot() {
return (
<div className="relative flex min-h-0 flex-1 flex-col">
{/* Top bar — stays in place */}
<div className="flex shrink-0 items-center p-1 border-b">
<Tabs
value={statusTab}
@@ -75,7 +73,6 @@ export default function NetworkRoot() {
</Tabs>
</div>
{/* Scrollable content */}
<div className="min-h-0 flex-1 overflow-y-auto overscroll-contain pb-14 py-2">
<ParticleListView
streams={streams}
js/desktop/src/lib/platform/electron.ts
+5
View File
@@ -55,4 +55,9 @@ export const electronPlatform: Platform = {
setDockBadge: (count) => window.electronApp.setDockBadge(count),
getVersion: () => window.electronApp.getVersion(),
},
deepLink: {
getPending: () => window.electronDeepLink.getPending(),
onNavigate: (cb) => window.electronDeepLink.onNavigate(cb),
},
};
js/desktop/src/lib/platform/types.ts
+5
View File
@@ -60,6 +60,11 @@ export interface Platform {
setDockBadge: (count: number) => void;
getVersion: () => Promise<string>;
};
deepLink: {
getPending: () => Promise<string | null>;
onNavigate: (callback: (path: string) => void) => () => void;
};
}
export const DESKTOP_DOWNLOAD_URL = 'https://flowylabs.ai/llink/download';
js/desktop/src/lib/platform/web.ts
+7
View File
@@ -117,4 +117,11 @@ export const webPlatform: Platform = {
setDockBadge: applyDockBadge,
getVersion: async () => __APP_VERSION__,
},
deepLink: {
getPending: () => Promise.resolve(null),
onNavigate: (_) => {
return () => {};
},
},
};