Skip to content

Commit 409a4d3

Browse files
lilyshen0722claude
andauthored
fix(cli): name a non-JSON error page by its title, not its whole body (#2092)
* fix(cli): name a non-JSON error page by its title, not its whole body A non-JSON error response put its entire body into err.message. When the Cloudflare tunnel could not reach the origin, every failed poll wrote a ~120-line HTML page into the agent log (10 pages in c4-smoke.log, 2026-09-22 to 09-29). An HTML body now becomes "HTTP <status>: <title>". Other plain text collapses to one line, capped at 300 characters. JSON error bodies are unchanged, and err.body keeps the full text for callers that classify it, so spawn-retry still sees every word. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013geJsV4RJMujdvpf3JV2Cv * test(cli): witness classification from the discarded page body Vera's #2092 probe: a 502 page whose only classifiable words sit deep in the body. The message drops them; err.body.message keeps them, and classifySpawnFailure must still read rate_limit. Removing body.message from spawn-retry's errorText fails this test and no other. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013geJsV4RJMujdvpf3JV2Cv * chore(cli): 0.1.85 for the error-message change cli/src changed, so the published version must rise ("Source changed ⇒ version bumped"). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013geJsV4RJMujdvpf3JV2Cv --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 949ac2a commit 409a4d3

4 files changed

Lines changed: 135 additions & 5 deletions

File tree

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
import { jest } from '@jest/globals';
2+
import os from 'os';
3+
import path from 'path';
4+
import fs from 'fs';
5+
6+
const configTmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-api-error-text-'));
7+
await jest.unstable_mockModule('os', () => ({
8+
...os,
9+
default: { ...os, homedir: () => configTmpDir },
10+
homedir: () => configTmpDir,
11+
}));
12+
13+
const { saveInstance } = await import('../src/lib/config.js');
14+
const { createClient } = await import('../src/lib/api.js');
15+
const { classifySpawnFailure, SPAWN_FAILURE_CLASS } = await import('../src/lib/spawn-retry.js');
16+
17+
afterAll(() => fs.rmSync(path.join(configTmpDir, '.commonly'), { recursive: true, force: true }));
18+
19+
beforeEach(() => {
20+
saveInstance({
21+
key: 'dev', url: 'https://api.commonly.me', token: 't', userId: 'u1', username: 'lily',
22+
});
23+
});
24+
25+
const respond = (status, text) => {
26+
global.fetch = jest.fn().mockResolvedValue({ ok: false, status, text: async () => text });
27+
};
28+
29+
const failure = async () => {
30+
try {
31+
await createClient({ instance: 'dev' }).get('/api/agents/runtime/events');
32+
} catch (err) {
33+
return err;
34+
}
35+
throw new Error('expected the request to reject');
36+
};
37+
38+
// The shape of the page Cloudflare serves when the tunnel cannot reach the origin.
39+
const tunnelPage = [
40+
'<!DOCTYPE html>',
41+
'<!--[if lt IE 7]> <html class="no-js ie6 oldie" lang="en-US"> <![endif]-->',
42+
'<html class="no-js" lang="en-US">',
43+
' <head>',
44+
' <title>Cloudflare Tunnel error | api.commonly.me | Cloudflare</title>',
45+
' </head>',
46+
' <body>',
47+
...Array.from({ length: 100 }, (_, i) => ` <div class="row-${i}">Cloudflare Tunnel error</div>`),
48+
' </body>',
49+
'</html>',
50+
].join('\n');
51+
52+
test('an HTML error page becomes one line named by its <title>', async () => {
53+
respond(502, tunnelPage);
54+
const err = await failure();
55+
expect(err.message).toBe('HTTP 502: Cloudflare Tunnel error | api.commonly.me | Cloudflare');
56+
expect(err.message).not.toContain('\n');
57+
expect(err.status).toBe(502);
58+
// Callers that classify the failure still see the whole page.
59+
expect(err.body.message).toBe(tunnelPage);
60+
});
61+
62+
test('a classifiable word only in the discarded page body still classifies', async () => {
63+
// 502, so the status === 429 shortcut cannot fire: the words exist only in
64+
// err.body.message, which the one-line message drops (Vera, #2092 review).
65+
const page = [
66+
'<!DOCTYPE html><html><head><title>Bad gateway</title></head><body>',
67+
...Array.from({ length: 80 }, (_, i) => `<div class="row-${i}">upstream</div>`),
68+
'<p>rate limit exceeded</p>',
69+
'</body></html>',
70+
].join('\n');
71+
respond(502, page);
72+
const err = await failure();
73+
expect(err.message).toBe('HTTP 502: Bad gateway');
74+
expect(err.message).not.toMatch(/rate limit/);
75+
expect(classifySpawnFailure(err)).toBe(SPAWN_FAILURE_CLASS.RATE_LIMIT);
76+
});
77+
78+
test('an HTML page with no <title> is named by its status', async () => {
79+
respond(503, '<html><body><h1>Service Unavailable</h1></body></html>');
80+
expect((await failure()).message).toBe('HTTP 503 (HTML error page)');
81+
});
82+
83+
test('a short plain-text error is kept verbatim', async () => {
84+
respond(429, 'Too many requests, please try again later.');
85+
const err = await failure();
86+
expect(err.message).toBe('Too many requests, please try again later.');
87+
expect(classifySpawnFailure(err)).toBe(SPAWN_FAILURE_CLASS.RATE_LIMIT);
88+
});
89+
90+
test('a long plain-text error is collapsed to one line and capped', async () => {
91+
const text = `upstream failed\n${'x'.repeat(1000)}`;
92+
respond(500, text);
93+
const err = await failure();
94+
expect(err.message).not.toContain('\n');
95+
expect(err.message).toHaveLength(301);
96+
expect(err.message.endsWith('…')).toBe(true);
97+
expect(err.body.message).toBe(text);
98+
});
99+
100+
test('an empty body falls back to the status', async () => {
101+
respond(500, '');
102+
expect((await failure()).message).toBe('HTTP 500');
103+
});
104+
105+
test('a JSON error body is unchanged', async () => {
106+
respond(403, JSON.stringify({ error: 'grant_not_found', code: 'grant_not_found' }));
107+
const err = await failure();
108+
expect(err.message).toBe('grant_not_found');
109+
expect(err.body.code).toBe('grant_not_found');
110+
});

‎cli/package-lock.json‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cli/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@commonlyai/cli",
3-
"version": "0.1.84",
3+
"version": "0.1.85",
44
"license": "Apache-2.0",
55
"description": "The Commonly CLI — connect agents, manage pods, iterate fast",
66
"type": "module",

‎cli/src/lib/api.js‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,34 @@ export const sessionExpiredMessage = ({ instanceKey, baseUrl }) => (
2020
`Session for ${instanceKey} (${baseUrl}) has expired.\nRun: commonly login --instance ${instanceKey}`
2121
);
2222

23+
// A non-JSON error body is usually an edge proxy's HTML page; a Cloudflare
24+
// tunnel error is ~120 lines, and the message used to be all of them, so every
25+
// failed poll dumped a whole page into the agent log. The message names the
26+
// page by its <title>; err.body keeps the full text for callers that classify it.
27+
const MAX_TEXT_ERROR = 300;
28+
const summarizeTextError = (text, status) => {
29+
const trimmed = String(text || '').trim();
30+
if (!trimmed) return null;
31+
if (/^<(?:!doctype|html)/i.test(trimmed) || /<html[\s>]/i.test(trimmed)) {
32+
const title = trimmed.match(/<title[^>]*>([\s\S]*?)<\/title>/i)?.[1]?.replace(/\s+/g, ' ').trim();
33+
return title ? `HTTP ${status}: ${title}` : `HTTP ${status} (HTML error page)`;
34+
}
35+
const line = trimmed.replace(/\s+/g, ' ');
36+
return line.length > MAX_TEXT_ERROR ? `${line.slice(0, MAX_TEXT_ERROR)}…` : line;
37+
};
38+
2339
const handleResponse = async (res, session = null) => {
2440
const text = await res.text();
2541
let body;
26-
try { body = JSON.parse(text); } catch { body = { message: text }; }
42+
let isJson = true;
43+
try { body = JSON.parse(text); } catch { body = { message: text }; isJson = false; }
2744
if (!res.ok) {
45+
const serverMessage = isJson
46+
? body?.error || body?.message || body?.msg
47+
: summarizeTextError(text, res.status);
2848
const msg = res.status === 401 && session && knownSessionFailure(body)
2949
? sessionExpiredMessage(session)
30-
: body?.error || body?.message || body?.msg || `HTTP ${res.status}`;
50+
: serverMessage || `HTTP ${res.status}`;
3151
const err = new Error(msg);
3252
err.status = res.status;
3353
err.body = body;

0 commit comments

Comments
 (0)