Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions apps/cli-e2e/src/tests/telemetry.e2e.test.ts
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
import { chmodSync, writeFileSync } from "node:fs";
import { chmodSync, readFileSync, writeFileSync } from "node:fs";
import { join } from "node:path";
import { describe, expect } from "vitest";
import { testBehaviour, testParity } from "./test-context.ts";
Expand DownExpand Up@@ -58,9 +58,12 @@ describe("telemetry", () => {
});

testBehaviour("handles corrupted config gracefully", async ({ run, workspace }) => {
writeFileSync(join(workspace.path, "telemetry.json"), "{{not valid json}}");
const telemetryPath = join(workspace.path, "telemetry.json");
writeFileSync(telemetryPath, "{{not valid json}}");
const result = await run(["telemetry", "status"]);
expect(result.exitCode).not.toBe(0);
expect(result.exitCode).toBe(0);
expect(result.stdout).toMatch(/Telemetry is (enabled|disabled)\./);
expect(() => JSON.parse(readFileSync(telemetryPath, "utf8"))).not.toThrow();
});

testParity(["telemetry", "status"]);
Expand Down
19 changes: 13 additions & 6 deletions apps/cli/src/shared/telemetry/consent.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,6 +4,14 @@ import type { ConsentState, TelemetryConfig } from "./types.ts";

export const getConfigDir = CliConfig.useSync((cliConfig) => cliConfig.supabaseHome);

function parseTelemetryConfig(content: string): TelemetryConfig | null {
try {
return JSON.parse(content) as TelemetryConfig;
} catch {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

failing here bricks commands like supabase start on corrupted local telemetry state,
so returning null lets the existing first run path regenerate telemetry.json :D

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for jumping on this and fixing the immediate crash. I agree with the core behavior here: malformed telemetry state should never crash unrelated CLI commands, and treating it as absent local state is the right outcome.

I opened a follow-up PR to align the implementation a bit more with the Effect style we’re using in the TypeScript CLI: #5412

The main changes are:

  • avoid raw JSON.parse for local state files and decode through EffectSchema.fromJsonString(...) instead
  • avoid as TelemetryConfig casts, since they can make structurally invalid JSON look valid to TypeScript
  • avoid null internally and use Option for absent config, while still outputting null at JSON/API boundaries where that is the expected serialized value

So the user-facing fix remains the same, but malformed JSON and valid-but-wrong-shape JSON are both handled safely and silently.

return null;
}
}

export const readTelemetryConfig = Effect.fnUntraced(
function* (configDir: string) {
const fs = yield* FileSystem.FileSystem;
Expand All@@ -12,7 +20,7 @@ export const readTelemetryConfig = Effect.fnUntraced(
const exists = yield* fs.exists(configPath);
if (!exists) return null;
const content = yield* fs.readFileString(configPath);
return JSON.parse(content) as TelemetryConfig;
return parseTelemetryConfig(content);
},
(effect) => Effect.orElseSucceed(effect, () => null),
);
Expand All@@ -24,11 +32,10 @@ export const writeTelemetryConfig = Effect.fnUntraced(function* (
const fs = yield* FileSystem.FileSystem;
const path = yield* Path.Path;
yield* fs.makeDirectory(configDir, { recursive: true, mode: 0o700 });
yield* fs.writeFileString(
path.join(configDir, "telemetry.json"),
JSON.stringify(config, null, 2),
{ mode: 0o600 },
);
const configPath = path.join(configDir, "telemetry.json");
const tmpPath = `${configPath}.tmp.${Date.now()}`;
yield* fs.writeFileString(tmpPath, JSON.stringify(config, null, 2), { mode: 0o600 });
yield* fs.rename(tmpPath, configPath);
}, Effect.orDie);

export const getEffectiveConsent = Effect.fnUntraced(function* (config: TelemetryConfig | null) {
Expand Down
29 changes: 28 additions & 1 deletion apps/cli/src/shared/telemetry/consent.unit.test.ts
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
import { describe, expect, it } from "@effect/vitest";
import { BunServices } from "@effect/platform-bun";
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import path from "node:path";
import { Effect, Layer } from "effect";
import { cliConfigLayer } from "../../next/config/cli-config.layer.ts";
import {
mockProjectContext,
mockRuntimeInfo,
processEnvLayer,
} from "../../../tests/helpers/mocks.ts";
import { getEffectiveConsent } from "./consent.ts";
import { getEffectiveConsent, readTelemetryConfig } from "./consent.ts";
import type { TelemetryConfig } from "./types.ts";

function makeConfig(consent: TelemetryConfig["consent"]): TelemetryConfig {
Expand DownExpand Up@@ -40,6 +44,14 @@ function emptyEnv() {
);
}

function makeTempDir(): string {
return mkdtempSync(path.join(tmpdir(), "supabase-consent-test-"));
}

function writeTelemetryFile(dir: string, content: string): void {
writeFileSync(path.join(dir, "telemetry.json"), content);
}

describe("getEffectiveConsent", () => {
it.live("returns denied when DO_NOT_TRACK=1", () =>
Effect.gen(function* () {
Expand DownExpand Up@@ -90,3 +102,18 @@ describe("getEffectiveConsent", () => {
}).pipe(Effect.provide(emptyEnv())),
);
});

describe("readTelemetryConfig", () => {
it.live("returns null for malformed JSON instead of throwing", () => {
const dir = makeTempDir();
writeTelemetryFile(dir, "");

return Effect.gen(function* () {
const config = yield* readTelemetryConfig(dir);
expect(config).toBeNull();
}).pipe(
Effect.provide(BunServices.layer),
Effect.ensuring(Effect.sync(() => rmSync(dir, { recursive: true, force: true }))),
);
});
});
18 changes: 17 additions & 1 deletion apps/cli/src/shared/telemetry/runtime.layer.unit.test.ts
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
import { describe, expect, it } from "@effect/vitest";
import { BunServices } from "@effect/platform-bun";
import { existsSync, mkdtempSync, rmSync } from "node:fs";
import { existsSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import path from "node:path";
import { Effect, Layer } from "effect";
Expand DownExpand Up@@ -79,4 +79,20 @@ describe("telemetryRuntimeLayer", () => {
Effect.ensuring(Effect.sync(() => rmSync(homeDir, { recursive: true, force: true }))),
);
});

it.live("treats a malformed telemetry.json as a fresh first run instead of crashing", () => {
const homeDir = makeTempDir();
const configPath = path.join(homeDir, "telemetry.json");
writeFileSync(configPath, "");

return Effect.gen(function* () {
const runtime = yield* TelemetryRuntime;
expect(runtime.consent).toBe("granted");
expect(runtime.isFirstRun).toBe(true);
expect(existsSync(configPath)).toBe(true);
}).pipe(
Effect.provide(buildLayer({ homeDir })),
Effect.ensuring(Effect.sync(() => rmSync(homeDir, { recursive: true, force: true }))),
);
});
});
Loading