-
Notifications
You must be signed in to change notification settings - Fork 496
fix(cli): stop telemetry hangs and errors on blocked networks #5880
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
base: develop
Are you sure you want to change the base?
Changes from all commits
baa3c5a
a96c6f5
e38cb95
c367a3e
77ce839
4246000
6133c2d
8bec921
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 |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import { Effect } from "effect"; | ||
| import { PostHog, type PostHogOptions } from "posthog-node"; | ||
|
|
||
| const delivered = { | ||
| status: 200, | ||
| text: () => Promise.resolve(""), | ||
| json: () => Promise.resolve({}), | ||
| }; | ||
|
|
||
| // posthog-node has no logger hook: delivery failures hit hardcoded | ||
| // console.error calls and multi-second retries, so report them as delivered. | ||
| export const fireAndForgetFetch: NonNullable<PostHogOptions["fetch"]> = async (url, options) => { | ||
| try { | ||
| const response = await globalThis.fetch(url, options); | ||
| return response.status >= 400 ? delivered : response; | ||
| } catch { | ||
| return delivered; | ||
| } | ||
| }; | ||
|
|
||
| export const scopedPosthogClient = (apiKey: string, host: string) => | ||
| Effect.acquireRelease( | ||
| Effect.sync( | ||
| () => | ||
| new PostHog(apiKey, { | ||
| host, | ||
| flushAt: 1, | ||
| flushInterval: 0, | ||
| requestTimeout: 2_000, | ||
| fetch: fireAndForgetFetch, | ||
| }), | ||
| ), | ||
| // Catch on the promise: Effect.promise turns rejections into defects, | ||
| // which escape Effect.ignore and fail the command. | ||
| (client) => Effect.promise(() => client._shutdown(5_000).catch(() => undefined)), | ||
|
Comment on lines
+21
to
+35
Member
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. issue one event can be in flight while a later exit event remains queued. Shutdown then performs two sequential 2-second requests (eg: The existing port-9 test rejects immediately and misses this case |
||
| ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| import { describe, expect, it } from "@effect/vitest"; | ||
| import { afterEach, vi } from "vitest"; | ||
| import { Effect } from "effect"; | ||
| import { PostHog } from "posthog-node"; | ||
| import { fireAndForgetFetch, scopedPosthogClient } from "./posthog-client.ts"; | ||
|
|
||
| const BATCH_URL = "https://eu.i.posthog.com/batch/"; | ||
| const BATCH_OPTIONS = { method: "POST" as const, headers: {}, body: "{}" }; | ||
|
|
||
| describe("fireAndForgetFetch", () => { | ||
| afterEach(() => { | ||
| vi.unstubAllGlobals(); | ||
| }); | ||
|
|
||
| it("passes successful responses through untouched", async () => { | ||
| vi.stubGlobal("fetch", async () => new Response(`{"status":1}`, { status: 200 })); | ||
|
|
||
| const response = await fireAndForgetFetch(BATCH_URL, BATCH_OPTIONS); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(await response.text()).toBe(`{"status":1}`); | ||
| }); | ||
|
|
||
| it("reports success when the network is unreachable", async () => { | ||
| vi.stubGlobal("fetch", async () => { | ||
| throw new Error("connect ECONNREFUSED"); | ||
| }); | ||
|
|
||
| const response = await fireAndForgetFetch(BATCH_URL, BATCH_OPTIONS); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(await response.text()).toBe(""); | ||
| expect(await response.json()).toEqual({}); | ||
| }); | ||
|
|
||
| it("reports success on error responses so the SDK never retries or logs", async () => { | ||
| vi.stubGlobal( | ||
| "fetch", | ||
| async () => new Response("Proxy Authentication Required", { status: 407 }), | ||
| ); | ||
|
|
||
| const response = await fireAndForgetFetch(BATCH_URL, BATCH_OPTIONS); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(await response.text()).toBe(""); | ||
| }); | ||
| }); | ||
|
|
||
| describe("scopedPosthogClient", () => { | ||
| it.live("captures and shuts down cleanly against an unreachable host", () => | ||
| Effect.gen(function* () { | ||
| const client = yield* scopedPosthogClient("phc_test", "http://127.0.0.1:9"); | ||
| expect(client).toBeInstanceOf(PostHog); | ||
| client.capture({ event: "verify_event", distinctId: "device-1" }); | ||
| }).pipe(Effect.scoped), | ||
| ); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.