Skip to content

fix(scraper): emit JSON errors from run under --json / --pretty #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
danishashko wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(scraper): emit JSON errors from run under --json / --pretty #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
50 changes: 33 additions & 17 deletions src/__tests__/commands/scraper.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ import {
format_heal_summary,
resume_and_poll,
handle_approve_scraper,
format_run_error,
} from '../../commands/scraper';

describe('commands/scraper', ()=>{
Expand Down Expand Up @@ -771,16 +772,11 @@ describe('commands/scraper', ()=>{

it('exits when trigger returns no response_id', async()=>{
mocks.post.mockResolvedValueOnce({});
const exit = vi.spyOn(process, 'exit')
.mockImplementation(()=>undefined as never);
const error = vi.spyOn(console, 'error')
.mockImplementation(()=>{});
await handle_run_scraper('c_abc', 'https://x.com', {});
const msg = error.mock.calls.map(c=>String(c[0])).join('\n');
await expect(handle_run_scraper('c_abc', 'https://x.com', {}))
.rejects.toThrow();
const msg = String(mocks.fail.mock.calls[0][0]);
expect(msg).toContain('response_id');
expect(msg).toContain('c_abc');
exit.mockRestore();
error.mockRestore();
});
});

Expand Down Expand Up @@ -820,17 +816,11 @@ describe('commands/scraper', ()=>{
stub_fetch(202,
JSON.stringify({error: 'crawl_results_timeout',
response_id: 'r_late'}));
const exit = vi.spyOn(process, 'exit')
.mockImplementation(()=>undefined as never);
const error = vi.spyOn(console, 'error')
.mockImplementation(()=>{});
await handle_run_scraper('c_abc', 'https://x.com',
{sync: true});
const msg = error.mock.calls.map(c=>String(c[0])).join('\n');
await expect(handle_run_scraper('c_abc', 'https://x.com',
{sync: true})).rejects.toThrow();
const msg = String(mocks.fail.mock.calls[0][0]);
expect(msg).toContain('r_late');
expect(msg).toContain('Re-run without --sync');
exit.mockRestore();
error.mockRestore();
});

it('honors --sync-timeout', async()=>{
Expand Down Expand Up @@ -2195,3 +2185,29 @@ describe('commands/scraper', ()=>{
});
});
});

describe('format_run_error', ()=>{
it('returns the raw message (no JSON) when no machine format is set',
()=>{
expect(format_run_error('boom', {})).toEqual(
{json: false, text: 'boom'});
});

it('returns a compact JSON error object under --json', ()=>{
expect(format_run_error('boom', {json: true})).toEqual(
{json: true, text: '{"error":"boom"}'});
});

it('strips the "Error:" prefix and extra lines in the JSON payload',
()=>{
expect(format_run_error('Error: bad thing\nstack', {json: true}))
.toEqual({json: true, text: '{"error":"bad thing"}'});
});

it('pretty-prints the JSON error under --pretty', ()=>{
expect(format_run_error('boom', {pretty: true})).toEqual({
json: true,
text: JSON.stringify({error: 'boom'}, null, 2),
});
});
});
97 changes: 64 additions & 33 deletions src/commands/scraper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,44 @@ const format_create_summary = (
const clean_error_message = (msg: string): string=>
msg.split('\n')[0].replace(/^Error:\s*/, '').trim();

// Error output for the `run` command. When the caller asked for machine
// output (--json / --pretty), a failed run must return a parseable error
// object on stdout, not a plain-text line on stderr, so an agent reading
// stdout still gets something it can JSON.parse. Without those flags the
// behaviour (a red line on stderr) is unchanged.
const format_run_error = (
msg: string,
opts: {json?: boolean; pretty?: boolean}
): {json: boolean; text: string}=>{
if (opts.json || opts.pretty)
{
const payload = {error: clean_error_message(msg)};
return {
json: true,
text: opts.pretty
? JSON.stringify(payload, null, 2)
: JSON.stringify(payload),
};
}
return {json: false, text: msg};
};

const run_fail = (
msg: string,
opts: {json?: boolean; pretty?: boolean}
): void=>{
const out = format_run_error(msg, opts);
if (out.json)
{
process.stdout.write(out.text+'\n');
process.exit(1);
return;
}
// Non-machine output: reuse the shared fail() (red ✗ line on stderr +
// exit 1), so every run error is formatted consistently.
fail(out.text);
};

const build_create_envelope = (params: {
collector_id: string;
name: string;
Expand Down Expand Up @@ -1033,7 +1071,7 @@ const run_batch = async(
try {
timeout = parse_timeout(timeout_raw);
} catch(e) {
fail((e as Error).message);
run_fail((e as Error).message, opts);
return;
}
if (reason == 'page_limit_fallback')
Expand All @@ -1060,10 +1098,9 @@ const run_batch = async(
trigger_spinner.stop();
if (!trigger.collection_id)
{
console.error(
run_fail(
`Failed to submit batch job (collector ${collector_id}): `
+'missing collection_id.');
process.exit(1);
+'missing collection_id.', opts);
return;
}
collection_id = trigger.collection_id;
Expand All @@ -1073,10 +1110,9 @@ const run_batch = async(
`Batch job: ${collection_id}${eta}`));
} catch(e) {
trigger_spinner.stop();
console.error(
run_fail(
`Failed to submit batch job (collector ${collector_id}): `
+`${(e as Error).message}`);
process.exit(1);
+`${(e as Error).message}`, opts);
return;
}
const poll_spinner = start_spinner('Collecting (batch)...');
Expand Down Expand Up @@ -1105,8 +1141,7 @@ const run_batch = async(
const msg = (e as Error).message;
const suffix = msg.includes(collection_id)
? '' : ` (collection_id ${collection_id})`;
console.error(`${msg}${suffix}`);
process.exit(1);
run_fail(`${msg}${suffix}`, opts);
return;
}
};
Expand All @@ -1121,17 +1156,18 @@ const handle_run_scraper = async(
try {
urls = resolve_run_inputs(url, opts);
} catch(e) {
fail((e as Error).message);
run_fail((e as Error).message, opts);
return;
}
if (urls.length > 1)
{
if (opts.sync)
{
fail(
run_fail(
'--sync cannot be combined with --urls / --input-file. '
+'The /dca/crawl endpoint accepts only a single URL. '
+'Drop --sync to use the batch endpoint (/dca/trigger).');
+'Drop --sync to use the batch endpoint (/dca/trigger).',
opts);
return;
}
await run_batch(api_key, collector_id, urls, opts, 'multi_url');
Expand All @@ -1144,7 +1180,7 @@ const handle_run_scraper = async(
try {
sync_timeout = parse_sync_timeout(opts.syncTimeout);
} catch(e) {
fail((e as Error).message);
run_fail((e as Error).message, opts);
return;
}
const query = build_run_query(collector_id, opts,
Expand All @@ -1162,21 +1198,19 @@ const handle_run_scraper = async(
const parsed = parse_result_body(res.body) as
{response_id?: string; message?: string};
const rid = parsed?.response_id ?? '<unknown>';
console.error(
run_fail(
`Sync request timed out server-side after `
+`${sync_timeout}s (response_id: ${rid}). `
+'Re-run without --sync to poll for results.'
);
process.exit(1);
+'Re-run without --sync to poll for results.',
opts);
return;
}
if (res.status < 200 || res.status >= 300)
{
console.error(
run_fail(
`Failed to scrape (collector ${collector_id}): `
+`HTTP ${res.status} ${res.body.slice(0, 200)}`
);
process.exit(1);
+`HTTP ${res.status} ${res.body.slice(0, 200)}`,
opts);
return;
}
const data = parse_result_body(res.body);
Expand All @@ -1190,18 +1224,17 @@ const handle_run_scraper = async(
return;
} catch(e) {
spinner.stop();
console.error(
run_fail(
`Failed to scrape (collector ${collector_id}): `
+`${(e as Error).message}`);
process.exit(1);
+`${(e as Error).message}`, opts);
return;
}
}
let timeout = 600;
try {
timeout = parse_timeout(opts.timeout);
} catch(e) {
fail((e as Error).message);
run_fail((e as Error).message, opts);
return;
}
const trigger_spinner = start_spinner('Triggering scrape...');
Expand All @@ -1217,20 +1250,18 @@ const handle_run_scraper = async(
trigger_spinner.stop();
if (!trigger.response_id)
{
console.error(
run_fail(
`Failed to trigger scraper (collector ${collector_id}): `
+'missing response_id in trigger response.');
process.exit(1);
+'missing response_id in trigger response.', opts);
return;
}
response_id = trigger.response_id;
console.error(dim(`Triggered (response_id: ${response_id})`));
} catch(e) {
trigger_spinner.stop();
console.error(
run_fail(
`Failed to trigger scraper (collector ${collector_id}): `
+`${(e as Error).message}`);
process.exit(1);
+`${(e as Error).message}`, opts);
return;
}
const poll_spinner = start_spinner('Waiting for results...');
Expand Down Expand Up @@ -1263,8 +1294,7 @@ const handle_run_scraper = async(
const msg = (e as Error).message;
const suffix = msg.includes(response_id)
? '' : ` (response_id ${response_id})`;
console.error(`${msg}${suffix}`);
process.exit(1);
run_fail(`${msg}${suffix}`, opts);
return;
}
};
Expand Down Expand Up @@ -1475,6 +1505,7 @@ export {
build_create_envelope,
emit_create_output,
handle_run_scraper,
format_run_error,
build_run_request,
build_run_query,
classify_result,
Expand Down