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
79 changes: 75 additions & 4 deletions src/commands/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,79 @@ export function parseFields(fields: string[]): Record<string, unknown> {
return result;
}

/**
* Convert a value to string, JSON-stringifying objects to avoid "[object Object]".
* @internal
*/
function stringifyValue(value: unknown): string {
if (typeof value === "object" && value !== null) {
return JSON.stringify(value);
}
return String(value);
}

/**
* Build query parameters from field strings for GET requests.
* Unlike parseFields(), this produces a flat structure suitable for URL query strings.
* Arrays are represented as string[] for repeated keys (e.g., tags=1&tags=2&tags=3).
*
* @param fields - Array of "key=value" strings
* @returns Record suitable for URLSearchParams
* @throws {Error} When field doesn't contain "="
* @internal Exported for testing
*/
export function buildQueryParams(
fields: string[]
): Record<string, string | string[]> {
const result: Record<string, string | string[]> = {};

for (const field of fields) {
const eqIndex = field.indexOf("=");
if (eqIndex === -1) {
throw new Error(`Invalid field format: ${field}. Expected key=value`);
}

const key = field.substring(0, eqIndex);
const rawValue = field.substring(eqIndex + 1);
const value = parseFieldValue(rawValue);

// Handle arrays by creating string[] for repeated keys
// Use stringifyValue to handle objects (avoid "[object Object]")
if (Array.isArray(value)) {
result[key] = value.map(stringifyValue);
} else {
result[key] = stringifyValue(value);
}
}

return result;
}

/**
* Prepare request options from command flags.
* Routes fields to either query params (GET) or request body (other methods).
*
* @param method - HTTP method
* @param fields - Optional array of "key=value" field strings
* @returns Object with body and params, one of which will be undefined
* @internal Exported for testing
*/
export function prepareRequestOptions(
method: HttpMethod,
fields?: string[]
): {
body?: Record<string, unknown>;
params?: Record<string, string | string[]>;
} {
const hasFields = fields && fields.length > 0;
const isBodyMethod = method !== "GET";

return {
body: hasFields && isBodyMethod ? parseFields(fields) : undefined,
params: hasFields && !isBodyMethod ? buildQueryParams(fields) : undefined,
};
}

/**
* Parse header arguments into headers object.
*
Expand Down Expand Up @@ -259,10 +332,7 @@ export const apiCommand = buildCommand({
): Promise<void> {
const { stdout } = this;

const body =
flags.field && flags.field.length > 0
? parseFields(flags.field)
: undefined;
const { body, params } = prepareRequestOptions(flags.method, flags.field);
const headers =
flags.header && flags.header.length > 0
? parseHeaders(flags.header)
Expand All @@ -271,6 +341,7 @@ export const apiCommand = buildCommand({
const response = await rawApiRequest(endpoint, {
method: flags.method,
body,
params,
headers,
});

Expand Down
19 changes: 15 additions & 4 deletions src/lib/api-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ function normalizePath(endpoint: string): string {
type ApiRequestOptions<T = unknown> = {
method?: "GET" | "POST" | "PUT" | "DELETE" | "PATCH";
body?: unknown;
params?: Record<string, string | number | boolean | undefined>;
/** Query parameters. String arrays create repeated keys (e.g., tags=1&tags=2) */
params?: Record<string, string | number | boolean | string[] | undefined>;
/** Optional Zod schema for runtime validation of response data */
schema?: z.ZodType<T>;
};
Expand Down Expand Up @@ -129,20 +130,30 @@ async function createApiClient(): Promise<KyInstance> {

/**
* Build URLSearchParams from an options object, filtering out undefined values.
* Supports string arrays for repeated keys (e.g., { tags: ["a", "b"] } → tags=a&tags=b).
*
* @param params - Key-value pairs to convert to search params
* @returns URLSearchParams instance, or undefined if no valid params
* @internal Exported for testing
*/
function buildSearchParams(
params?: Record<string, string | number | boolean | undefined>
export function buildSearchParams(
params?: Record<string, string | number | boolean | string[] | undefined>
): URLSearchParams | undefined {
if (!params) {
return;
}

const searchParams = new URLSearchParams();
for (const [key, value] of Object.entries(params)) {
if (value !== undefined) {
if (value === undefined) {
continue;
}
if (Array.isArray(value)) {
// Repeated keys for arrays: tags=1&tags=2&tags=3
for (const item of value) {
searchParams.append(key, item);
}
} else {
searchParams.set(key, String(value));
}
}
Expand Down
143 changes: 143 additions & 0 deletions test/commands/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@

import { describe, expect, test } from "bun:test";
import {
buildQueryParams,
parseFields,
parseFieldValue,
parseHeaders,
parseMethod,
prepareRequestOptions,
setNestedValue,
} from "../../src/commands/api.js";

Expand Down Expand Up @@ -192,3 +194,144 @@ describe("parseHeaders", () => {
expect(parseHeaders([])).toEqual({});
});
});

describe("buildQueryParams", () => {
test("builds simple key=value params", () => {
expect(buildQueryParams(["status=resolved", "limit=10"])).toEqual({
status: "resolved",
limit: "10",
});
});

test("handles arrays as repeated keys", () => {
expect(buildQueryParams(["tags=[1,2,3]"])).toEqual({
tags: ["1", "2", "3"],
});
});

test("handles arrays of strings", () => {
expect(buildQueryParams(['names=["alice","bob"]'])).toEqual({
names: ["alice", "bob"],
});
});

test("converts all values to strings", () => {
expect(buildQueryParams(["count=42", "active=true", "value=null"])).toEqual(
{
count: "42",
active: "true",
value: "null",
}
);
});

test("handles value with equals sign", () => {
expect(buildQueryParams(["query=a=b"])).toEqual({ query: "a=b" });
});

test("throws for invalid field format", () => {
expect(() => buildQueryParams(["invalid"])).toThrow(/Invalid field format/);
expect(() => buildQueryParams(["no-equals"])).toThrow(
/Invalid field format/
);
});

test("returns empty object for empty array", () => {
expect(buildQueryParams([])).toEqual({});
});

test("handles objects by JSON stringifying them", () => {
expect(buildQueryParams(['data={"key":"value"}'])).toEqual({
data: '{"key":"value"}',
});
});

test("handles nested objects by JSON stringifying them", () => {
expect(buildQueryParams(['filter={"user":{"name":"john"}}'])).toEqual({
filter: '{"user":{"name":"john"}}',
});
});

test("handles arrays of objects by JSON stringifying each element", () => {
expect(
buildQueryParams(['filters=[{"key":"value"},{"key2":"value2"}]'])
).toEqual({
filters: ['{"key":"value"}', '{"key2":"value2"}'],
});
});

test("handles mixed arrays with objects and primitives", () => {
expect(buildQueryParams(['data=[1,{"obj":true},"string"]'])).toEqual({
data: ["1", '{"obj":true}', "string"],
});
});
});

describe("prepareRequestOptions", () => {
test("GET with no fields returns undefined for both body and params", () => {
const result = prepareRequestOptions("GET", undefined);
expect(result.body).toBeUndefined();
expect(result.params).toBeUndefined();
});

test("GET with empty fields returns undefined for both body and params", () => {
const result = prepareRequestOptions("GET", []);
expect(result.body).toBeUndefined();
expect(result.params).toBeUndefined();
});

test("GET with fields returns params (not body)", () => {
const result = prepareRequestOptions("GET", [
"status=resolved",
"limit=10",
]);
expect(result.body).toBeUndefined();
expect(result.params).toEqual({
status: "resolved",
limit: "10",
});
});

test("POST with fields returns body (not params)", () => {
const result = prepareRequestOptions("POST", ["status=resolved"]);
expect(result.body).toEqual({ status: "resolved" });
expect(result.params).toBeUndefined();
});

test("PUT with fields returns body (not params)", () => {
const result = prepareRequestOptions("PUT", ["name=test"]);
expect(result.body).toEqual({ name: "test" });
expect(result.params).toBeUndefined();
});

test("PATCH with fields returns body (not params)", () => {
const result = prepareRequestOptions("PATCH", ["active=true"]);
expect(result.body).toEqual({ active: true });
expect(result.params).toBeUndefined();
});

test("DELETE with fields returns body (not params)", () => {
const result = prepareRequestOptions("DELETE", ["force=true"]);
expect(result.body).toEqual({ force: true });
expect(result.params).toBeUndefined();
});

test("POST with no fields returns undefined for both body and params", () => {
const result = prepareRequestOptions("POST", undefined);
expect(result.body).toBeUndefined();
expect(result.params).toBeUndefined();
});

test("GET with array field converts to string array in params", () => {
const result = prepareRequestOptions("GET", ["tags=[1,2,3]"]);
expect(result.params).toEqual({ tags: ["1", "2", "3"] });
});

test("POST with nested fields creates nested body object", () => {
const result = prepareRequestOptions("POST", [
"user.name=John",
"user.age=30",
]);
expect(result.body).toEqual({ user: { name: "John", age: 30 } });
});
});
46 changes: 46 additions & 0 deletions test/e2e/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,4 +161,50 @@ describe("sentry api", () => {
},
{ timeout: 15_000 }
);

test(
"GET request with --field uses query parameters (not body)",
async () => {
await setAuthToken(TEST_TOKEN);

// Use issues endpoint with query parameter - this tests that --field
// with GET request properly converts fields to query params instead of body
// (GET requests cannot have a body, so this would fail if fields went to body)
const result = await runCli(
["api", "projects/", "--field", "query=platform:javascript"],
{
env: { [CONFIG_DIR_ENV_VAR]: testConfigDir },
}
);

// Should succeed (not throw "GET/HEAD method cannot have body" error)
expect(result.exitCode).toBe(0);
const data = JSON.parse(result.stdout);
expect(Array.isArray(data)).toBe(true);
},
{ timeout: 15_000 }
);

test(
"POST request with --field uses request body",
async () => {
await setAuthToken(TEST_TOKEN);

// POST to a read-only endpoint will return 405, but the important thing
// is that it doesn't fail with a client-side error about body/params
const result = await runCli(
["api", "organizations/", "--method", "POST", "--field", "name=test"],
{
env: { [CONFIG_DIR_ENV_VAR]: testConfigDir },
}
);

// Should get a server error (405 Method Not Allowed or 400 Bad Request),
// not a client-side error about body handling
expect(result.exitCode).toBe(1);
// The error should be from the API, not a TypeError about body
expect(result.stdout + result.stderr).not.toMatch(/cannot have body/i);
},
{ timeout: 15_000 }
);
});
Loading
Loading