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
53 changes: 2 additions & 51 deletions packages/cli/src/commands/auth/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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<void> {
const listed = listOrganizationsUncached().then(
() => true,
(error: unknown) => {
log.debug("Could not warm the org cache after login", error);
return false;
},
);
let timer: ReturnType<typeof setTimeout> | undefined;
const timedOut = new Promise<false>((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();
}
5 changes: 5 additions & 0 deletions packages/cli/src/commands/cli/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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.");
}
Expand Down
50 changes: 47 additions & 3 deletions packages/cli/src/lib/completion-warmup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<void> {
const listed = listOrganizationsUncached().then(
() => true,
(error: unknown) => {
log.debug("Could not warm the org cache after login", error);
return false;
},
);
let timer: ReturnType<typeof setTimeout> | undefined;
const timedOut = new Promise<false>((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();
}
135 changes: 28 additions & 107 deletions packages/cli/test/commands/auth/login.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -731,119 +728,43 @@ 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([]);
getCurrentUserSpy.mockResolvedValue(SAMPLE_USER);
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 () => {
Expand Down
32 changes: 31 additions & 1 deletion packages/cli/test/commands/cli/setup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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 {
Expand Down Expand Up @@ -965,6 +975,7 @@ describe("sentry cli setup", () => {
existingBinary?: boolean;
install?: boolean;
method?: string;
completions?: boolean;
} = {},
) {
const sourcePath = join(testDir, "sentry-download");
Expand Down Expand Up @@ -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 ?? []),
],
Expand Down Expand Up @@ -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();
Expand Down
Loading
Loading