Skip to content
Draft
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
89 changes: 88 additions & 1 deletion integration/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,89 @@ async function placesAnswering(...answers: Answer[]) {
}

describe("error classes", () => {
it.each([
{ method: "POST", status: 403 },
{ method: "POST", status: 401 },
{ method: "GET", status: 401 },
{ method: "GET", status: 404 },
{ method: "GET", status: 429 },
{ method: "GET", status: 503 },
])("reports a $method $status edge challenge without login, expiry or retry advice", async ({
method,
status,
}) => {
signIn(s.home);
const requests: { method: string; path: string; token: string | null }[] =
[];
const edge = Bun.serve({
hostname: "127.0.0.1",
port: 0,
fetch(req) {
requests.push({
method: req.method,
path: new URL(req.url).pathname,
token: req.headers.get("authorization"),
});
return new Response("<html><title>Just a moment...</title></html>", {
status,
headers: {
"content-type": "text/html",
"cf-mitigated": "challenge",
"cf-ray": "a4d0830ba9d11e2d-SIN",
"retry-after": "0",
},
});
},
});
try {
for (let attempt = 0; attempt < 2; attempt++) {
const result = await s.run(
method === "POST"
? [
"hotels",
"search",
"AUH",
"2027-01-17",
"2027-01-22",
"--adults",
"2",
"--children",
"1",
"--children-ages",
"10",
"--rooms",
"1",
]
: ["places", "London"],
{
env: {
WEGO_API_URL: String(edge.url),
WEGO_AUTH_TOKEN_URL: new URL("/oauth/token", edge.url).href,
},
},
);
expect(result.code).toBe(6);
expect(result.out).toBe("");
expect(result.err).toContain("Cloudflare challenged");
expect(result.err).toContain("cf_ray=a4d0830ba9d11e2d-SIN");
expect(result.err).toContain("feedback");
expect(result.err).not.toContain("wego login");
expect(result.err).not.toContain("trace_id=");
expect(result.err).not.toContain("<html>");
expect(result.err).not.toContain("expired");
}
expect(requests).toEqual(
Array.from({ length: 2 }, () => ({
method,
path: method === "POST" ? "/v1/hotels/searches" : "/v1/places",
token: "Bearer access-1",
})),
);
} finally {
edge.stop(true);
}
});

it("a persistent 401 with no refresh token exits 3 and names login", async () => {
signIn(s.home, { accessToken: "access-revoked" });
s.fake({ accept: [] });
Expand Down Expand Up @@ -49,12 +132,16 @@ describe("error classes", () => {

it("a 400 exits 6 and prints the API's detail and trace id", async () => {
const { result } = await placesAnswering(
problem(400, "validation_failed", "query is too short"),
problem(400, "validation_failed", "query is too short", {
"cf-ray": "api-ray-SIN",
}),
);
expect(result.code).toBe(6);
expect(result.out).toBe("");
expect(result.err).toContain("query is too short");
expect(result.err).toContain("trace-400");
expect(result.err).toContain("cf_ray=api-ray-SIN");
expect(result.err).not.toContain("Cloudflare challenged");
});

it("a 429 is retried after Retry-After, then succeeds", async () => {
Expand Down
61 changes: 61 additions & 0 deletions src/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,67 @@ afterAll(() => {
globalThis.fetch = realFetch;
});

describe("edge error metadata", () => {
it.each(
[401, 403, 404, 429, 503].map((status) => ({ status })),
)("identifies marked $status challenges without retrying and keeps edge metadata", async ({
status,
}) => {
for (const ray of [undefined, "a4d0830ba9d11e2d-SIN"]) {
let calls = 0;
const http = (async () => {
calls++;
return new Response("<html>Just a moment...</html>", {
status,
headers: {
"content-type": "text/html",
"cf-mitigated": "challenge",
"retry-after": "0",
...(ray ? { "cf-ray": ray } : {}),
},
});
}) as HttpFetch;
const err = await fetchWhoami(
"https://api.wego.com",
"tok",
undefined,
http,
).catch((e) => e);
expect(err).toBeInstanceOf(ApiHttpError);
expect(err.edgeChallenge).toBe(true);
expect(err.rayId).toBe(ray);
expect(err.traceId).toBeUndefined();
expect(err.code).toBeUndefined();
expect(calls).toBe(1);
}
});

it("does not classify ordinary API errors behind Cloudflare as challenges", async () => {
const http = (async () =>
new Response(
JSON.stringify({ code: "insufficient_scope", trace_id: "body-trace" }),
{
status: 403,
headers: {
"content-type": "application/problem+json",
"cf-ray": "ordinary-ray",
"x-trace-id": "api-trace",
},
},
)) as HttpFetch;
const err = await fetchWhoami(
"https://api.wego.com",
"tok",
undefined,
http,
).catch((e) => e);
expect(err.edgeChallenge).toBe(false);
expect(err.rayId).toBe("ordinary-ray");
expect(err.traceId).toBe("api-trace");
expect(err.code).toBe("insufficient_scope");
});
});

describe("USER_AGENT", () => {
it("is the Wego-CLI product token, so apps/api can classify the caller", () => {
expect(USER_AGENT).toMatch(/^Wego-CLI\/\S+$/);
Expand Down
14 changes: 14 additions & 0 deletions src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ export interface ApiHttpErrorFields {
code?: string;
detail?: string;
traceId?: string;
edgeChallenge?: boolean;
rayId?: string;
retryAfterSeconds?: number;
bodyParseError?: boolean;
}
Expand All @@ -115,6 +117,8 @@ export class ApiHttpError extends Error {
readonly code?: string;
readonly detail?: string;
readonly traceId?: string;
readonly edgeChallenge: boolean;
readonly rayId?: string;
readonly retryAfterSeconds?: number;
readonly bodyParseError: boolean;

Expand All @@ -130,6 +134,8 @@ export class ApiHttpError extends Error {
this.code = fields.code;
this.detail = fields.detail;
this.traceId = fields.traceId;
this.edgeChallenge = fields.edgeChallenge ?? false;
this.rayId = fields.rayId;
this.retryAfterSeconds = fields.retryAfterSeconds;
this.bodyParseError = fields.bodyParseError ?? false;
}
Expand Down Expand Up @@ -195,6 +201,8 @@ async function readApiError(
code: parsed.code,
detail: parsed.detail,
traceId: res.headers.get("x-trace-id") ?? parsed.traceId,
edgeChallenge: res.headers.get("cf-mitigated") === "challenge",
rayId: res.headers.get("cf-ray") ?? undefined,
retryAfterSeconds: parseRetryAfter(res.headers.get("retry-after")),
bodyParseError: parsed.bodyParseError,
});
Expand Down Expand Up @@ -355,6 +363,9 @@ async function authedJsonGet<T>(
},
http,
);
if (res.headers.get("cf-mitigated") === "challenge") {
throw await readApiError(res, label);
}
if (res.status === 401) throw new UnauthorizedError();
if (res.status === 404) throw new NotFoundError(label);
if (!res.ok) {
Expand Down Expand Up @@ -409,6 +420,9 @@ async function authedJsonPost<T>(
},
http,
);
if (res.headers.get("cf-mitigated") === "challenge") {
throw await readApiError(res, label);
}
if (res.status === 401) throw new UnauthorizedError();
if (!res.ok) {
// No auto-retry on POST: a create is not idempotent, so a blind retry could
Expand Down
7 changes: 7 additions & 0 deletions src/error-report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const EXIT_BY_PROBLEM_CODE = new Map<string, number>(
* binary was compiled before. Split out of `exitCodeForError` to keep that
* dispatcher within the cognitive-complexity gate. */
function exitCodeForHttpError(err: ApiHttpError): number {
if (err.edgeChallenge) return EXIT.PERMANENT;

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.

🟡 Bundled skill misclassifies challenged 403 responses

A challenged 403 now exits 6, but the agent exit-code table assigns every code-less 403 to exit 3. The shipped agent guidance remains wrong for the response this change targets.

Learn more

The embedded skill is the published operating contract for agents. Its exit-code table tells agents that any 403 without an API code exits 3 and prescribes login, whereas marked 403 challenges are now deliberately emitted as exit 6.

Example: A hotel-search challenge has status 403 and no problem code. The CLI exits 6, while the bundled table predicts exit 3 and says to log in.

Recommended fix: Update the embedded skill's exit-code table to distinguish a cf-mitigated: challenge response from an ordinary 403. Keep the table consistent with the actual CLI taxonomy and the repository's rules governing changes to skills/.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

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.

Confirmed. The bundled exit-code table still classifies every code-less 401/403 as auth, every 404 as not-found and every 429/503 as retryable. A response explicitly marked cf-mitigated: challenge is now an exception: exit 6, preserve cf_ray, report the challenge, and do not log in again, refresh, re-search or automatically retry.

I left this finding open because AGENTS.md explicitly prohibits editing skills/. The repository owner needs to update the bundled guidance or explicitly authorize that narrow edit. The mismatch is also called out in the PR's Notes for reviewers.

The transport findings are fixed in the follow-up commit, with compiled-binary regressions covering the challenge marker before auth, not-found and retry shortcuts.

const byCode =
err.code === undefined ? undefined : EXIT_BY_PROBLEM_CODE.get(err.code);
if (byCode !== undefined) return byCode;
Expand Down Expand Up @@ -116,6 +117,7 @@ function formatHttpError(err: ApiHttpError, prog: string): string {
const parts = [err.message];
if (err.bodyParseError) parts.push("(unparseable error body)");
if (err.traceId) parts.push(`trace_id=${err.traceId}`);
if (err.rayId) parts.push(`cf_ray=${err.rayId}`);
if (err.retryAfterSeconds != null) {
parts.push(`retry after ${err.retryAfterSeconds}s`);
}
Expand Down Expand Up @@ -156,6 +158,11 @@ function rateLimitRemedy(err: ApiHttpError, prog: string): string[] {
* the remedy is already right.
*/
function authRemedy(err: ApiHttpError, prog: string): string[] {
if (err.edgeChallenge) {
return [
`Cloudflare challenged this request before it reached the API; logging in again will not solve the challenge. Report this with \`${prog} feedback --message "..."\` and include the cf_ray if shown`,
];
}
if (err.code === "insufficient_scope") {
return [
`not authorized for this operation – \`${prog} login\` would re-request the same scopes, so ask for this access to be granted to your Wego account`,
Expand Down
Loading