-
Notifications
You must be signed in to change notification settings - Fork 970
refactor: clean up Outlit SDK integration #1374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8734063
3c7f013
e67ab6a
51626eb
1deca92
d4aac0f
e3b14b4
1fc0ff4
36a682e
b049fd7
1b06971
6362160
3b9d80e
f65082b
1cc4077
1678be4
c9fab41
4e6dd2c
12c6b58
b407c4b
80bfd83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -23,6 +23,8 @@ export const env = createEnv({ | |||||
| NEXT_PUBLIC_POSTHOG_KEY: z.string().optional(), | ||||||
| NEXT_PUBLIC_POSTHOG_HOST: z.string().default("https://us.i.posthog.com"), | ||||||
| SENTRY_DSN_DESKTOP: z.string().optional(), | ||||||
| STREAMS_URL: z.url().default("https://superset-stream.fly.dev"), | ||||||
| NEXT_PUBLIC_OUTLIT_KEY: z.string(), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Prompt for AI agents
Suggested change
|
||||||
| }, | ||||||
|
|
||||||
| runtimeEnv: { | ||||||
|
|
@@ -37,6 +39,8 @@ export const env = createEnv({ | |||||
| NEXT_PUBLIC_POSTHOG_KEY: process.env.NEXT_PUBLIC_POSTHOG_KEY, | ||||||
| NEXT_PUBLIC_POSTHOG_HOST: process.env.NEXT_PUBLIC_POSTHOG_HOST, | ||||||
| SENTRY_DSN_DESKTOP: process.env.SENTRY_DSN_DESKTOP, | ||||||
| STREAMS_URL: process.env.STREAMS_URL, | ||||||
| NEXT_PUBLIC_OUTLIT_KEY: process.env.NEXT_PUBLIC_OUTLIT_KEY, | ||||||
| }, | ||||||
| emptyStringAsUndefined: true, | ||||||
| // Only allow skipping validation in development (never in production) | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -28,6 +28,7 @@ import { setupAutoUpdater } from "./lib/auto-updater"; | |||||||||||||||
| import { setWorkspaceDockIcon } from "./lib/dock-icon"; | ||||||||||||||||
| import { loadWebviewBrowserExtension } from "./lib/extensions"; | ||||||||||||||||
| import { localDb } from "./lib/local-db"; | ||||||||||||||||
| import { outlit } from "./lib/outlit"; | ||||||||||||||||
| import { ensureProjectIconsDir, getProjectIconPath } from "./lib/project-icons"; | ||||||||||||||||
| import { initSentry } from "./lib/sentry"; | ||||||||||||||||
| import { | ||||||||||||||||
|
|
@@ -187,7 +188,10 @@ app.on("before-quit", async (event) => { | |||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // Quit confirmed or no confirmation needed - exit immediately | ||||||||||||||||
| // Let OS clean up child processes, tray, etc. | ||||||||||||||||
| isQuitting = true; | ||||||||||||||||
| await outlit.shutdown(); | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If the SDK call throws or never resolves, 🛡️ Proposed fix- await outlit.shutdown();
+ await Promise.race([
+ outlit.shutdown().catch((err) =>
+ console.error("[main] Outlit shutdown failed:", err),
+ ),
+ new Promise((resolve) => setTimeout(resolve, 2000)),
+ ]);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Unhandled rejection in (Based on your team's feedback about handling async calls with proper await and catch.) Prompt for AI agents |
||||||||||||||||
| disposeTray(); | ||||||||||||||||
| app.exit(0); | ||||||||||||||||
| }); | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| import { app } from "electron"; | ||
| import { env } from "main/env.main"; | ||
| import { outlit } from "main/lib/outlit"; | ||
| import { PostHog } from "posthog-node"; | ||
| import { toOutlitProperties } from "shared/analytics"; | ||
| import { DEFAULT_TELEMETRY_ENABLED } from "shared/constants"; | ||
|
|
||
| export let posthog: PostHog | null = null; | ||
|
|
@@ -37,16 +39,27 @@ export function track( | |
| if (!isTelemetryEnabled()) return; | ||
|
|
||
| const client = getClient(); | ||
| if (!client) return; | ||
|
|
||
| client.capture({ | ||
| distinctId: userId, | ||
| event, | ||
| properties: { | ||
| ...properties, | ||
| app_name: "desktop", | ||
| platform: process.platform, | ||
| desktop_version: app.getVersion(), | ||
| }, | ||
| if (client) { | ||
| client.capture({ | ||
| distinctId: userId, | ||
| event, | ||
| properties: { | ||
| ...properties, | ||
| app_name: "desktop", | ||
| platform: process.platform, | ||
| desktop_version: app.getVersion(), | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| outlit.track({ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Outlit tracking is missing the enriched properties ( Prompt for AI agents |
||
| eventName: event, | ||
| userId, | ||
| properties: toOutlitProperties(properties), | ||
| }); | ||
|
|
||
| // Fire user.activate() on project_opened (activation moment) | ||
| if (event === "project_opened") { | ||
| outlit.user.activate({ userId }); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| import { Outlit } from "@outlit/node"; | ||
| import { env } from "main/env.main"; | ||
|
|
||
| export const outlit = new Outlit({ | ||
| publicKey: env.NEXT_PUBLIC_OUTLIT_KEY, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import { useEffect } from "react"; | ||
| import { electronTrpc } from "renderer/lib/electron-trpc"; | ||
| import { outlit } from "renderer/lib/outlit"; | ||
| import { posthog } from "renderer/lib/posthog"; | ||
|
|
||
| export function TelemetrySync() { | ||
|
|
@@ -9,21 +10,16 @@ export function TelemetrySync() { | |
| useEffect(() => { | ||
| if (telemetryEnabled === undefined) return; | ||
|
|
||
| try { | ||
| if (telemetryEnabled) { | ||
| if (typeof posthog?.opt_in_capturing === "function") { | ||
| posthog.opt_in_capturing(); | ||
| } | ||
| } else { | ||
| if (typeof posthog?.opt_out_capturing === "function") { | ||
| posthog.opt_out_capturing(); | ||
| } | ||
| if (telemetryEnabled) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Removing the (Based on your team's feedback about handling async calls with proper await and catch.) Prompt for AI agents |
||
| if (typeof posthog?.opt_in_capturing === "function") { | ||
| posthog.opt_in_capturing(); | ||
| } | ||
| } catch (error) { | ||
| console.error( | ||
| "[telemetry-sync] Failed to update telemetry state:", | ||
| error, | ||
| ); | ||
| outlit.enableTracking(); | ||
| } else { | ||
| if (typeof posthog?.opt_out_capturing === "function") { | ||
| posthog.opt_out_capturing(); | ||
| } | ||
| outlit.disableTracking(); | ||
| } | ||
| }, [telemetryEnabled]); | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Using a repository secret directly for this required env var makes
pull_requestruns from forks unreliable, because fork-triggered workflows do not receive secrets. Add a safe fallback value for CI (or gate secret-dependent steps) so external PR checks remain runnable.Prompt for AI agents