diff --git a/packages/cli/src/commands/auth/login.ts b/packages/cli/src/commands/auth/login.ts index b5d6b4829..66ce49633 100644 --- a/packages/cli/src/commands/auth/login.ts +++ b/packages/cli/src/commands/auth/login.ts @@ -2,11 +2,7 @@ import { isatty } from "node:tty"; import { isSaaSTrustOrigin } from "@sentry/toolkit-core/sentry-origin"; import type { SentryContext } from "../../context.js"; import { pointDefaultUrlAtAccount } from "../../lib/account-selection.js"; -import { - getCurrentUser, - getUserRegions, - listOrganizationsUncached, -} from "../../lib/api-client.js"; +import { getCurrentUser, getUserRegions } from "../../lib/api-client.js"; import { normalizeAuthToken } from "../../lib/auth-header.js"; import { buildCommand, numberParser } from "../../lib/command.js"; import { @@ -15,10 +11,7 @@ import { completingParser, parseText, } from "../../lib/completion-target.js"; -import { - WARMUP_TIMEOUT_MS, - warmAttributeCompletions, -} from "../../lib/completion-warmup.js"; +import { warmCachesAfterLogin } from "../../lib/completion-warmup.js"; import { normalizeUrl } from "../../lib/constants.js"; import { getAccountIdToReplace, @@ -747,45 +740,3 @@ export const loginCommand = buildCommand({ process.exitCode = 1; }, }); - -/** - * Warm the caches the next commands read, after a successful login. - * - * First the org + region cache, so the first real command doesn't pay the - * cold-start cost of `getUserRegions()` + fan-out to each region's org list - * endpoint (~800ms on a typical SaaS account). Then, with the account's orgs - * known, the attribute keys that field completion offers (see - * {@link warmAttributeCompletions}). - * - * Login has already printed success, so each step gets - * {@link WARMUP_TIMEOUT_MS}. An org list that takes longer is left to finish - * in the background, as before this warm-up existed, and the attribute - * warm-up is skipped. - * - * Never throws: each cache is populated lazily by the next command that - * needs it anyway. - */ -async function warmCachesAfterLogin(): Promise { - const listed = listOrganizationsUncached().then( - () => true, - (error: unknown) => { - log.debug("Could not warm the org cache after login", error); - return false; - }, - ); - let timer: ReturnType | undefined; - const timedOut = new Promise((resolve) => { - timer = setTimeout(() => { - log.debug("Org list is still loading; not warming attribute keys"); - resolve(false); - }, WARMUP_TIMEOUT_MS); - }); - try { - if (!(await Promise.race([listed, timedOut]))) { - return; - } - } finally { - clearTimeout(timer); - } - await warmAttributeCompletions(); -} diff --git a/packages/cli/src/commands/cli/setup.ts b/packages/cli/src/commands/cli/setup.ts index 60602a9fd..99a4bde8c 100644 --- a/packages/cli/src/commands/cli/setup.ts +++ b/packages/cli/src/commands/cli/setup.ts @@ -31,6 +31,7 @@ import { import { SETUP_REFRESH_AFTER_MS, warmAttributeCompletions, + warmCachesAfterLogin, } from "../../lib/completion-warmup.js"; import { CLI_VERSION } from "../../lib/constants.js"; import { getAuthConfig } from "../../lib/db/auth.js"; @@ -848,6 +849,10 @@ export const setupCommand = buildCommand({ const result = await runInteractiveLogin(); if (result) { log.success("Authenticated with Sentry."); + // The warm-up above ran before this login stored a token. + if (!flags["no-completions"]) { + await warmCachesAfterLogin(); + } } else { log.info("Run 'sentry auth login' to authenticate later."); } diff --git a/packages/cli/src/lib/completion-warmup.ts b/packages/cli/src/lib/completion-warmup.ts index d1cc6141a..a267946ec 100644 --- a/packages/cli/src/lib/completion-warmup.ts +++ b/packages/cli/src/lib/completion-warmup.ts @@ -8,14 +8,17 @@ * (see `api/completion-seed.ts`), so without a warm-up field completion * offers nothing until the user runs `sentry attribute list`. * - * `auth login` and `cli setup` (which `cli upgrade` runs) call - * {@link warmAttributeCompletions}. It requests each dataset of a few orgs in - * parallel and lets the fetch layer store the keys. It is best-effort: it + * `cli setup` (which `cli upgrade` runs) calls + * {@link warmAttributeCompletions}; `auth login` and the login at the end of + * a fresh install call {@link warmCachesAfterLogin}. The warm-up requests + * each dataset of a few orgs in parallel and lets the fetch layer store the + * keys. It is best-effort: it * never throws or prompts, logs only at debug level, and gives up after * {@link WARMUP_TIMEOUT_MS} so the command never looks stuck. */ import { ATTRIBUTE_KINDS } from "./api/completion-seed.js"; +import { listOrganizationsUncached } from "./api-client.js"; import { listTraceItemAttributes } from "./api/traces.js"; import { hasStoredAuthCredentials } from "./db/auth.js"; import { @@ -150,3 +153,44 @@ export async function warmAttributeCompletions( clearTimeout(timer); } } + +/** + * Warm the caches the next commands read, after a successful login. + * + * First the org + region cache, so the first real command doesn't pay the + * cold-start cost of `getUserRegions()` + fan-out to each region's org list + * endpoint (~800ms on a typical SaaS account). Then, with the account's orgs + * known, the attribute keys that field completion offers (see + * {@link warmAttributeCompletions}). + * + * Login has already printed success, so each step gets + * {@link WARMUP_TIMEOUT_MS}. An org list that takes longer is left to finish + * in the background and the attribute warm-up is skipped. + * + * Never throws: each cache is populated lazily by the next command that + * needs it anyway. + */ +export async function warmCachesAfterLogin(): Promise { + const listed = listOrganizationsUncached().then( + () => true, + (error: unknown) => { + log.debug("Could not warm the org cache after login", error); + return false; + }, + ); + let timer: ReturnType | undefined; + const timedOut = new Promise((resolve) => { + timer = setTimeout(() => { + log.debug("Org list is still loading; not warming attribute keys"); + resolve(false); + }, WARMUP_TIMEOUT_MS); + }); + try { + if (!(await Promise.race([listed, timedOut]))) { + return; + } + } finally { + clearTimeout(timer); + } + await warmAttributeCompletions(); +} diff --git a/packages/cli/test/commands/auth/login.test.ts b/packages/cli/test/commands/auth/login.test.ts index 31b13af22..8fc12a53d 100644 --- a/packages/cli/test/commands/auth/login.test.ts +++ b/packages/cli/test/commands/auth/login.test.ts @@ -152,18 +152,15 @@ import { ValidationError, } from "../../../src/lib/errors.js"; -vi.mock("../../../src/lib/completion-warmup.js", async (importOriginal) => { - const actual = - await importOriginal< - typeof import("../../../src/lib/completion-warmup.js") - >(); - return Object.fromEntries( - Object.entries(actual).map(([k, v]) => [ - k, - typeof v === "function" ? vi.fn(v) : v, - ]), - ); -}); +// The post-login warm-up makes its own API requests; tests/lib cover it. +vi.mock("../../../src/lib/completion-warmup.js", async (importOriginal) => ({ + ...(await importOriginal< + typeof import("../../../src/lib/completion-warmup.js") + >()), + warmCachesAfterLogin: vi.fn(async () => { + // no-op + }), +})); // oxlint-disable-next-line sentry-cli/no-namespace-import -- needed for spyOn mocking import * as completionWarmup from "../../../src/lib/completion-warmup.js"; @@ -731,7 +728,7 @@ describe("loginCommand.func --token path", () => { expect(setUserInfoSpy).not.toHaveBeenCalled(); }); - test("--token: warms attribute completions after the org list, once the result is printed", async () => { + test("--token: warms caches once the result is printed", async () => { isAuthenticatedSpy.mockReturnValue(false); setAuthTokenSpy.mockReturnValue(undefined); getUserRegionsSpy.mockResolvedValue([]); @@ -739,111 +736,35 @@ describe("loginCommand.func --token path", () => { setUserInfoSpy.mockReturnValue(undefined); const { context, getStdout } = createContext(); let stdoutAtWarmup: string | undefined; - const warmSpy = vi - .spyOn(completionWarmup, "warmAttributeCompletions") - .mockImplementation(async () => { - stdoutAtWarmup = getStdout(); - }); + const warm = vi.mocked(completionWarmup.warmCachesAfterLogin); + warm.mockClear(); + warm.mockImplementationOnce(async () => { + stdoutAtWarmup = getStdout(); + }); - try { - await func.call(context, { - token: "my-token", - force: false, - timeout: 900, - }); + await func.call(context, { + token: "my-token", + force: false, + timeout: 900, + }); - expect(listOrgsUncachedSpy).toHaveBeenCalledTimes(1); - expect(warmSpy).toHaveBeenCalledTimes(1); - expect(warmSpy).toHaveBeenCalledWith(); - // Ordering: the org list resolves before the warm-up picks orgs - expect(listOrgsUncachedSpy.mock.invocationCallOrder[0]).toBeLessThan( - warmSpy.mock.invocationCallOrder[0]!, - ); - expect(stdoutAtWarmup).toContain("Authenticated"); - } finally { - warmSpy.mockRestore(); - } + expect(warm).toHaveBeenCalledTimes(1); + expect(stdoutAtWarmup).toContain("Authenticated"); }); - test("OAuth: warms attribute completions after a successful login", async () => { + test("OAuth: warms caches after a successful login", async () => { isAuthenticatedSpy.mockReturnValue(false); runInteractiveLoginSpy.mockResolvedValue({ method: "oauth", configPath: "/tmp/db", }); - const warmSpy = vi - .spyOn(completionWarmup, "warmAttributeCompletions") - .mockResolvedValue(undefined); - - try { - const { context } = createContext(); - await func.call(context, { force: false, timeout: 900 }); - - expect(warmSpy).toHaveBeenCalledTimes(1); - } finally { - warmSpy.mockRestore(); - } - }); + const warm = vi.mocked(completionWarmup.warmCachesAfterLogin); + warm.mockClear(); - test("a stalled org list skips the attribute warm-up after the timeout", async () => { - isAuthenticatedSpy.mockReturnValue(false); - setAuthTokenSpy.mockReturnValue(undefined); - getUserRegionsSpy.mockResolvedValue([]); - getCurrentUserSpy.mockResolvedValue(SAMPLE_USER); - setUserInfoSpy.mockReturnValue(undefined); - listOrgsUncachedSpy.mockReturnValue( - new Promise(() => { - // never settles - }), - ); - const warmSpy = vi - .spyOn(completionWarmup, "warmAttributeCompletions") - .mockResolvedValue(undefined); - - try { - const { context, getStdout } = createContext(); - const started = Date.now(); - await func.call(context, { - token: "my-token", - force: false, - timeout: 900, - }); - - expect(Date.now() - started).toBeLessThan( - completionWarmup.WARMUP_TIMEOUT_MS + 1000, - ); - expect(getStdout()).toContain("Authenticated"); - expect(warmSpy).not.toHaveBeenCalled(); - } finally { - warmSpy.mockRestore(); - } - }); - - test("a failed org list skips the attribute warm-up without failing login", async () => { - isAuthenticatedSpy.mockReturnValue(false); - setAuthTokenSpy.mockReturnValue(undefined); - getUserRegionsSpy.mockResolvedValue([]); - getCurrentUserSpy.mockResolvedValue(SAMPLE_USER); - setUserInfoSpy.mockReturnValue(undefined); - listOrgsUncachedSpy.mockRejectedValue(new Error("offline")); - const warmSpy = vi - .spyOn(completionWarmup, "warmAttributeCompletions") - .mockResolvedValue(undefined); - - try { - const { context, getStdout } = createContext(); - await func.call(context, { - token: "my-token", - force: false, - timeout: 900, - }); + const { context } = createContext(); + await func.call(context, { force: false, timeout: 900 }); - expect(getStdout()).toContain("Authenticated"); - expect(warmSpy).not.toHaveBeenCalled(); - expect(process.exitCode ?? 0).toBe(0); - } finally { - warmSpy.mockRestore(); - } + expect(warm).toHaveBeenCalledTimes(1); }); test("no token: falls through to interactive login", async () => { diff --git a/packages/cli/test/commands/cli/setup.test.ts b/packages/cli/test/commands/cli/setup.test.ts index 0e2d43103..d5815d50c 100644 --- a/packages/cli/test/commands/cli/setup.test.ts +++ b/packages/cli/test/commands/cli/setup.test.ts @@ -43,6 +43,15 @@ vi.mock("../../../src/lib/scope-recovery.js", async (importOriginal) => ({ ensureCurrentOAuthScopes: vi.fn(), })); +vi.mock("../../../src/lib/completion-warmup.js", async (importOriginal) => ({ + ...(await importOriginal< + typeof import("../../../src/lib/completion-warmup.js") + >()), + warmCachesAfterLogin: vi.fn(async () => { + // no-op: covered by test/lib/completion-warmup.test.ts + }), +})); + vi.mock("../../../src/lib/windows-path.js", async (importOriginal) => ({ ...(await importOriginal< typeof import("../../../src/lib/windows-path.js") @@ -51,6 +60,7 @@ vi.mock("../../../src/lib/windows-path.js", async (importOriginal) => ({ })); import { app } from "../../../src/app.js"; +import { warmCachesAfterLogin } from "../../../src/lib/completion-warmup.js"; import type { SentryContext } from "../../../src/context.js"; import { getAuthConfig, setAuthToken } from "../../../src/lib/db/auth.js"; import { @@ -965,6 +975,7 @@ describe("sentry cli setup", () => { existingBinary?: boolean; install?: boolean; method?: string; + completions?: boolean; } = {}, ) { const sourcePath = join(testDir, "sentry-download"); @@ -1002,7 +1013,7 @@ describe("sentry cli setup", () => { "--method", options.method ?? "curl", "--no-modify-path", - "--no-completions", + ...(options.completions ? [] : ["--no-completions"]), "--no-agent-skills", ...(options.flags ?? []), ], @@ -1030,6 +1041,25 @@ describe("sentry cli setup", () => { expect(fixture.getOutput()).toContain("Authenticated with Sentry."); }); + test.each([ + [true, 1], + [false, 0], + ])( + "warms caches for the new login when completions=%s", + async (completions, calls) => { + vi.mocked(interactiveLogin.runInteractiveLogin).mockResolvedValue({ + method: "oauth", + configPath: getConfigDir(), + }); + vi.mocked(warmCachesAfterLogin).mockClear(); + + await createInstall({ completions }).run(); + + expect(interactiveLogin.runInteractiveLogin).toHaveBeenCalledOnce(); + expect(warmCachesAfterLogin).toHaveBeenCalledTimes(calls); + }, + ); + test.each([0, 1, 2])("skips login when fd %i is not a TTY", async (fd) => { vi.mocked(isatty).mockImplementation((candidate) => candidate !== fd); const fixture = createInstall(); diff --git a/packages/cli/test/lib/completion-warmup.test.ts b/packages/cli/test/lib/completion-warmup.test.ts index 1f2291c51..b652fcb13 100644 --- a/packages/cli/test/lib/completion-warmup.test.ts +++ b/packages/cli/test/lib/completion-warmup.test.ts @@ -5,6 +5,8 @@ */ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +// oxlint-disable-next-line sentry-cli/no-namespace-import -- needed for spyOn mocking +import * as apiClient from "../../src/lib/api-client.js"; import { getCompletions } from "../../src/lib/complete.js"; import { MAX_RECENT_ORGS, @@ -12,6 +14,7 @@ import { RECENT_ORG_WINDOW_MS, WARMUP_TIMEOUT_MS, warmAttributeCompletions, + warmCachesAfterLogin, } from "../../src/lib/completion-warmup.js"; import { setAuthToken } from "../../src/lib/db/auth.js"; import { @@ -248,3 +251,53 @@ describe("warmAttributeCompletions", () => { }); }); }); + +describe("warmCachesAfterLogin", () => { + beforeEach(() => { + setAuthToken("test-token"); + mockAttributes(async () => Response.json([])); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + test("lists orgs, then warms the orgs that list cached", async () => { + vi.spyOn(apiClient, "listOrganizationsUncached").mockImplementation( + async () => { + seedOrgs("acme"); + return []; + }, + ); + + await warmCachesAfterLogin(); + + expect(new Set(requests.map((r) => r.org))).toEqual(new Set(["acme"])); + expect(requests).toHaveLength(3); + }); + + test("skips the attribute warm-up when the org list fails", async () => { + vi.spyOn(apiClient, "listOrganizationsUncached").mockRejectedValue( + new Error("offline"), + ); + seedOrgs("acme"); + + await expect(warmCachesAfterLogin()).resolves.toBeUndefined(); + expect(requests).toEqual([]); + }); + + test("skips the attribute warm-up when the org list stalls", async () => { + vi.spyOn(apiClient, "listOrganizationsUncached").mockReturnValue( + new Promise(() => { + // never settles + }), + ); + seedOrgs("acme"); + + const started = Date.now(); + await expect(warmCachesAfterLogin()).resolves.toBeUndefined(); + + expect(Date.now() - started).toBeLessThan(WARMUP_TIMEOUT_MS + 1000); + expect(requests).toEqual([]); + }); +});