Skip to content
Open
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
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ LINEAR_API_TOKEN=<token> linearis issues list # via environment variable

Token resolution order: `--api-token` flag → `LINEAR_API_TOKEN` env → `~/.linearis/token` → `~/.linear_api_token` (deprecated).

GraphQL requests time out after 30 seconds by default. Override the timeout for one command with `--graphql-timeout-ms`, or set `LINEAR_GRAPHQL_TIMEOUT_MS` for all commands in an environment. The flag takes precedence over the environment variable.

```bash
linearis --graphql-timeout-ms 5000 issues list
LINEAR_GRAPHQL_TIMEOUT_MS=5000 linearis auth status
```
Comment on lines +69 to +74

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Accurate per attempt, but it reads as a per-command promise, and that's where people will get bitten.

isRetryable() matches on "timed out", which is exactly what we throw on abort at graphql-client.ts:159. So a timeout is itself retryable: four attempts, plus 500ms + 1s + 2s of backoff. Someone sets 5000 wanting a fast fail in a script and waits about 23 seconds.

The retry behaviour is fine and predates this PR. It's the sentence that needs to be honest.

Suggested change
GraphQL requests time out after 30 seconds by default. Override the timeout for one command with `--graphql-timeout-ms`, or set `LINEAR_GRAPHQL_TIMEOUT_MS` for all commands in an environment. The flag takes precedence over the environment variable.
```bash
linearis --graphql-timeout-ms 5000 issues list
LINEAR_GRAPHQL_TIMEOUT_MS=5000 linearis auth status
```
Each request attempt times out after 30 seconds by default. Override it for one command with `--request-timeout-ms`, or set `LINEAR_REQUEST_TIMEOUT_MS` for all commands in an environment. The flag takes precedence over the environment variable.
Failed attempts are retried up to 3 times with backoff, so a command's total runtime can exceed the configured timeout.
```bash
linearis --request-timeout-ms 5000 issues list
LINEAR_REQUEST_TIMEOUT_MS=5000 linearis auth status
```


## Usage

All output is JSON. Start with discovery, then act.
Expand Down
12 changes: 10 additions & 2 deletions src/client/graphql-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,15 @@ import { withRetry } from "../common/retry.js";
const LINEAR_GRAPHQL_ENDPOINT = "https://api.linear.app/graphql";

/** Default timeout for GraphQL API requests (30 seconds) */
const REQUEST_TIMEOUT_MS = 30_000;
const DEFAULT_GRAPHQL_TIMEOUT_MS = 30_000;

let graphqlRequestTimeoutMs = DEFAULT_GRAPHQL_TIMEOUT_MS;

export function setGraphqlRequestTimeoutMs(
timeoutMs = DEFAULT_GRAPHQL_TIMEOUT_MS,
): void {
graphqlRequestTimeoutMs = timeoutMs;
}

/**
* Variable-less operations generate `Exact<{ [key: string]: never }>` for their
Expand Down Expand Up @@ -131,7 +139,7 @@ export class GraphQLClient {
const timeoutController = new AbortController();
const timeoutHandle = setTimeout(() => {
timeoutController.abort();
}, REQUEST_TIMEOUT_MS);
}, graphqlRequestTimeoutMs);

try {
// `TVariables extends Record<string, unknown>` lets `variables` widen
Expand Down
25 changes: 18 additions & 7 deletions src/commands/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import {
resolveApiToken,
type TokenSource,
} from "../common/auth.js";
import { createGraphQLClient, getRootOpts } from "../common/context.js";
import {
configureGraphqlRequestTimeout,
createGraphQLClient,
getRootOpts,
} from "../common/context.js";
import { handleCommand, outputSuccess } from "../common/output.js";
import { clearToken, saveToken } from "../common/token-storage.js";
import { type DomainMeta, formatDomainUsage } from "../common/usage.js";
Expand All @@ -29,6 +33,7 @@ export const AUTH_META: DomainMeta = {
"linearis requires a Linear API token for all operations.",
"the auth command guides you through creating and storing a token.",
"tokens are encrypted and stored in ~/.linearis/token.",
"graphql timeout order: --graphql-timeout-ms flag, LINEAR_GRAPHQL_TIMEOUT_MS env, default 30000ms.",
"token resolution order: --api-token flag, LINEAR_API_TOKEN env,",
"~/.linearis/token (encrypted), ~/.linear_api_token (deprecated).",
].join("\n"),
Expand Down Expand Up @@ -119,14 +124,21 @@ export function setupAuthCommands(program: Command): void {
.option("--force", "reauthenticate even if already authenticated")
.action(async (options: { force?: boolean }, command: Command) => {
try {
const rootOpts = getRootOpts(command) as CommandOptions;
configureGraphqlRequestTimeout(rootOpts);
if (!options.force) {
let existingToken: ReturnType<typeof resolveApiToken> | undefined;
try {
const rootOpts = getRootOpts(command) as CommandOptions;
const { token, source } = resolveApiToken(rootOpts);
existingToken = resolveApiToken(rootOpts);
} catch {
// No token found anywhere, proceed with login
}

if (existingToken) {
try {
const viewer = await validateApiToken(token);
const viewer = await validateApiToken(existingToken.token);
console.error(
`Already authenticated as ${viewer.name} (${viewer.email}) via ${SOURCE_LABELS[source]}.`,
`Already authenticated as ${viewer.name} (${viewer.email}) via ${SOURCE_LABELS[existingToken.source]}.`,
);
console.error("Run with --force to reauthenticate.");
return;
Expand All @@ -136,8 +148,6 @@ export function setupAuthCommands(program: Command): void {
"Existing token is invalid. Starting new authentication...",
);
}
} catch {
// No token found anywhere, proceed with login
}
}

Expand Down Expand Up @@ -217,6 +227,7 @@ export function setupAuthCommands(program: Command): void {
return;
}

configureGraphqlRequestTimeout(rootOpts);
try {
const viewer = await validateApiToken(token);
outputSuccess({
Expand Down
17 changes: 17 additions & 0 deletions src/common/auth.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { parseGraphqlTimeoutOption } from "./number-options.js";
import { getStoredToken } from "./token-storage.js";

export interface CommandOptions {
apiToken?: string;
compact?: boolean;
fields?: string[];
graphqlTimeoutMs?: number;
}

export type TokenSource = "flag" | "env" | "stored" | "legacy";
Expand Down Expand Up @@ -55,3 +57,18 @@ export function getApiToken(options: CommandOptions): string {
const { token } = resolveApiToken(options);
return token;
}

export function resolveGraphqlTimeoutMs(
options: CommandOptions,
): number | undefined {
if (options.graphqlTimeoutMs !== undefined) {
return options.graphqlTimeoutMs;
}

const environmentValue = process.env["LINEAR_GRAPHQL_TIMEOUT_MS"];
if (environmentValue) {
return parseGraphqlTimeoutOption(environmentValue);
}
Comment on lines +68 to +71

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Paired change for the parser above, so the env var names itself in the error.

Suggested change
const environmentValue = process.env["LINEAR_GRAPHQL_TIMEOUT_MS"];
if (environmentValue) {
return parseGraphqlTimeoutOption(environmentValue);
}
const environmentValue = process.env["LINEAR_REQUEST_TIMEOUT_MS"];
if (environmentValue) {
return parseTimeoutMs(environmentValue, "LINEAR_REQUEST_TIMEOUT_MS");
}


return undefined;
}
16 changes: 14 additions & 2 deletions src/common/context.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,27 @@
import type { Command } from "commander";
import { GraphQLClient } from "../client/graphql-client.js";
import { type CommandOptions, getApiToken } from "./auth.js";
import {
GraphQLClient,
setGraphqlRequestTimeoutMs,
} from "../client/graphql-client.js";
import {
type CommandOptions,
getApiToken,
resolveGraphqlTimeoutMs,
} from "./auth.js";

export type { CommandOptions };

export interface CommandContext {
gql: GraphQLClient;
}

export function configureGraphqlRequestTimeout(options: CommandOptions): void {
setGraphqlRequestTimeoutMs(resolveGraphqlTimeoutMs(options));
}

export function createContext(options: CommandOptions): CommandContext {
const token = getApiToken(options);
Comment on lines +18 to 23

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not blocking.

I read your note about keeping this process-global rather than threading it through the client API, and I'm fine with that. It matches setOutputOptions and it kept the diff small.

Where it gets called is what I'd push on. setOutputOptions fires from one place, the preAction hook in main.ts, with a comment on it saying "set once per process." This one fires from three, two of them inside command bodies. A missed fourth would be silent: the command just runs on the default and nobody notices.

Suggested change
export function configureGraphqlRequestTimeout(options: CommandOptions): void {
setGraphqlRequestTimeoutMs(resolveGraphqlTimeoutMs(options));
}
export function createContext(options: CommandOptions): CommandContext {
const token = getApiToken(options);
export function configureRequestTimeout(options: CommandOptions): void {
setRequestTimeoutMs(resolveRequestTimeoutMs(options));
}
export function createContext(options: CommandOptions): CommandContext {
const token = getApiToken(options);

with the hook in main.ts becoming (outside this diff, so no suggestion):

program.hook("preAction", async (_thisCommand, actionCommand) => {
  const rootOpts = getRootOpts(actionCommand);
  setOutputOptions(rootOpts);
  configureRequestTimeout(rootOpts);
  await maybeNotifyUpdate(pkg.version);
});

Both configureGraphqlRequestTimeout calls in commands/auth.ts can then go. I checked that preAction fires for nested subcommands with root options populated, so it covers auth login (which bypasses handleCommand) as well as auth status, and a throw still lands in parseAsync().catch(handleParseFailure) and comes out as JSON.

One trade-off, since you wrote a test pinning the current behaviour: running the hook first flips the precedence in context.test.ts. A malformed timeout gets reported before a missing token instead of after. I think that's fine, both are config errors and first-one-wins is a reasonable rule, but it's your call. Which is why this isn't blocking.

configureGraphqlRequestTimeout(options);
return {
gql: new GraphQLClient(token),
};
Expand Down
18 changes: 18 additions & 0 deletions src/common/number-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,21 @@ export function parseEstimateOption(raw: string): number {

return value;
}

export function parseGraphqlTimeoutOption(raw: string): number {
const value = parseStrictNonNegativeInteger(raw);
if (value === null || value < 1) {
throw invalidParameterError(
"--graphql-timeout-ms",
"must be a positive integer",
);
}
if (value > 2_147_483_647) {
throw invalidParameterError(
"--graphql-timeout-ms",
"must not exceed 2147483647",
);
}

return value;
}
Comment on lines +33 to +49

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

resolveGraphqlTimeoutMs runs the env var through this same parser, so LINEAR_GRAPHQL_TIMEOUT_MS=abc reports Invalid --graphql-timeout-ms: must be a positive integer. In CI that variable is set three layers up in a workflow file and the flag isn't in the command at all, so people will go looking in the wrong place.

Worth folding into the rename pass.

One snag I hit trying it: a defaulted source parameter doesn't compile. Commander passes the previous value as the second argument to an option parser and the types collide. Hence the thin wrapper below.

I lifted the timer bound into a constant while I was in there. 2147483647 is a good catch and the reason for it should sit next to the number, not only in the test name.

Suggested change
export function parseGraphqlTimeoutOption(raw: string): number {
const value = parseStrictNonNegativeInteger(raw);
if (value === null || value < 1) {
throw invalidParameterError(
"--graphql-timeout-ms",
"must be a positive integer",
);
}
if (value > 2_147_483_647) {
throw invalidParameterError(
"--graphql-timeout-ms",
"must not exceed 2147483647",
);
}
return value;
}
/**
* Node clamps `setTimeout` delays above 2^31-1 ms to 1 ms and warns, which
* would silently turn a very large timeout into an immediate failure.
*/
const MAX_TIMER_DELAY_MS = 2_147_483_647;
/**
* `source` names where the value came from, so an invalid environment variable
* is not reported as an invalid flag the user never passed.
*/
export function parseTimeoutMs(raw: string, source: string): number {
const value = parseStrictNonNegativeInteger(raw);
if (value === null || value < 1) {
throw invalidParameterError(source, "must be a positive integer");
}
if (value > MAX_TIMER_DELAY_MS) {
throw invalidParameterError(
source,
`must not exceed ${MAX_TIMER_DELAY_MS}`,
);
}
return value;
}
/**
* Commander option parser. Kept single-argument: Commander passes the previous
* value as the second argument, so `source` cannot be an optional parameter.
*/
export function parseTimeoutOption(raw: string): number {
return parseTimeoutMs(raw, "--request-timeout-ms");
}

On the built CLI that gives:

$ LINEAR_REQUEST_TIMEOUT_MS=abc linearis issues list
{ "error": "Invalid LINEAR_REQUEST_TIMEOUT_MS: must be a positive integer" }

$ linearis --request-timeout-ms 0 issues list
{ "error": "Invalid --request-timeout-ms: must be a positive integer" }

3 changes: 3 additions & 0 deletions src/common/usage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ export function formatOverview(version: string, metas: DomainMeta[]): string {
lines.push(
"auth: linearis auth login | --api-token <token> | LINEAR_API_TOKEN | ~/.linearis/token",
);
lines.push(
"graphql timeout: --graphql-timeout-ms <ms> | LINEAR_GRAPHQL_TIMEOUT_MS | default 30000",
);
lines.push("output: JSON");
lines.push("ids: UUID or human-readable (team key, issue ABC-123, name)");
lines.push("");
Expand Down
6 changes: 6 additions & 0 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
interceptParseErrors,
} from "./common/cli-errors.js";
import { getRootOpts } from "./common/context.js";
import { parseGraphqlTimeoutOption } from "./common/number-options.js";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The import goes with the rename.

Suggested change
import { parseGraphqlTimeoutOption } from "./common/number-options.js";
import { parseTimeoutOption } from "./common/number-options.js";

import { parseFieldsList, setOutputOptions } from "./common/output.js";
import { maybeNotifyUpdate } from "./common/update-notifier.js";
import {
Expand All @@ -46,6 +47,11 @@ program
.description("CLI for Linear.app with JSON output")
.version(pkg.version)
.option("--api-token <token>", "Linear API token")
.option(
"--graphql-timeout-ms <ms>",
"GraphQL request timeout in milliseconds",
parseGraphqlTimeoutOption,
)
Comment on lines +50 to +54

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

GraphQL is how we happen to talk to Linear. It isn't something a person using this CLI should need to know about, and nothing else on the surface mentions it: --api-token, --compact, --fields, LINEAR_API_TOKEN. The file-download path isn't GraphQL either, so the name is already a bit untrue.

Can we call it --request-timeout-ms and LINEAR_REQUEST_TIMEOUT_MS? REQUEST_TIMEOUT_MS was the internal constant name before this branch, so the rename mostly puts things back where they were.

Nothing is released yet. That means this costs a find-and-replace today, and a deprecation cycle if we leave it. It's the only reason I'm not just approving.

Suggested change
.option(
"--graphql-timeout-ms <ms>",
"GraphQL request timeout in milliseconds",
parseGraphqlTimeoutOption,
)
.option(
"--request-timeout-ms <ms>",
"per-attempt request timeout in milliseconds (default 30000)",
parseTimeoutOption,
)

Everything else is mechanical: resolveGraphqlTimeoutMs, setGraphqlRequestTimeoutMs, DEFAULT_GRAPHQL_TIMEOUT_MS, CommandOptions.graphqlTimeoutMs, and the test expectations.

.option("--compact", "emit single-line JSON (no indentation)")
.option(
"--fields <list>",
Expand Down
35 changes: 34 additions & 1 deletion tests/unit/client/graphql-client.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import type { TypedDocumentNode } from "@graphql-typed-document-node/core";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { GraphQLClient } from "../../../src/client/graphql-client.js";
import {
GraphQLClient,
setGraphqlRequestTimeoutMs,
} from "../../../src/client/graphql-client.js";
import { AuthenticationError } from "../../../src/common/errors.js";

// A stand-in document for the error-path tests. Typing its variables as
Expand Down Expand Up @@ -39,6 +42,7 @@ describe("GraphQLClient", () => {
let mockFetch: ReturnType<typeof vi.fn>;

beforeEach(() => {
setGraphqlRequestTimeoutMs();
mockFetch = vi.fn();
vi.stubGlobal("fetch", mockFetch);
});
Expand Down Expand Up @@ -222,6 +226,35 @@ describe("GraphQLClient", () => {
}
});

it("uses the configured request timeout", async () => {
vi.useFakeTimers();
try {
mockFetch.mockImplementation(
(_url: string, options: { signal?: AbortSignal }) =>
new Promise((_, reject) => {
options.signal?.addEventListener("abort", () => {
reject(new Error("This operation was aborted"));
});
}),
);

setGraphqlRequestTimeoutMs(1250);
const client = new GraphQLClient("good-token");
const promise = client.request(fakeDocument());
const rejection = expect(promise).rejects.toThrow("Request timed out");

await vi.advanceTimersByTimeAsync(1249);
expect(mockFetch.mock.calls[0]?.[1].signal.aborted).toBe(false);
await vi.advanceTimersByTimeAsync(1);
expect(mockFetch.mock.calls[0]?.[1].signal.aborted).toBe(true);

await vi.runAllTimersAsync();
await rejection;
} finally {
vi.useRealTimers();
}
});

it("retries on 429 and succeeds on next attempt", async () => {
mockFetch
.mockResolvedValueOnce(fakeResponse({ ok: false, status: 429 }, {}))
Expand Down
Loading