fix(client): surface HTTP status, bound requests, and type API errors

src/__tests__/commands/snapshot-readiness.test.ts

@@ -0,0 +1,73 @@
+import {describe, it, expect} from 'vitest';
+import {snapshot_running_status} from '../../commands/dataset';
+import type {Response_envelope} from '../../utils/client';
+
+// The snapshot endpoint answers in two dimensions that vary independently:
+// the HTTP status (200 data vs 202 still-building) and the body shape (a
+// parsed object under --format json, raw text under csv/ndjson/jsonl). Every
+// combination has to resolve correctly, because the failure that matters is
+// silent: a not-ready response mistaken for data prints a status stub and
+// exits 0, which downstream ETL then consumes as if it were the dataset.
+const envelope = (status: number, body: unknown): Response_envelope=>({
+    status,
+    headers: new Headers(),
+    body,
+});
+
+describe('commands/dataset.snapshot_running_status', ()=>{
+    it('200 + object data is ready', ()=>{
+        expect(snapshot_running_status(envelope(200, [{a: 1}])))
+            .toBeUndefined();
+    });
+
+    it('200 + text data is ready (csv/jsonl formats)', ()=>{
+        expect(snapshot_running_status(envelope(200, 'a,b\n1,2\n')))
+            .toBeUndefined();
+    });
+
+    it('200 + object status body is still running', ()=>{
+        expect(snapshot_running_status(envelope(200, {status: 'running'})))
+            .toBe('running');
+    });
+
+    it('200 + TEXT status body is still running', ()=>{
+        // The regression this predicate exists for: under a non-json format
+        // the client hands back a string, so an object-only check misses it
+        // and the status stub gets printed as data.
+        expect(snapshot_running_status(
+            envelope(200, '{"status":"running"}'))).toBe('running');
+    });
+
+    it('202 is still running even when the body looks like data', ()=>{
+        // The protocol is the most authoritative signal available.
+        expect(snapshot_running_status(envelope(202, [{a: 1}])))
+            .toBe('building');
+    });
+
+    it('202 + text status body reports the real status', ()=>{
+        expect(snapshot_running_status(
+            envelope(202, '{"status":"starting"}'))).toBe('starting');
+    });
+
+    it('keeps the real status string rather than a flattened literal', ()=>{
+        // Progress output prints this value, so collapsing every running
+        // state to one token would lose starting -> building -> running.
+        for (const s of ['starting', 'building', 'running'])
+        {
+            expect(snapshot_running_status(envelope(200, {status: s})))
+                .toBe(s);
+        }
+    });
+
+    it('treats terminal statuses as ready, not running', ()=>{
+        expect(snapshot_running_status(envelope(200, {status: 'ready'})))
+            .toBeUndefined();
+        expect(snapshot_running_status(envelope(200, {status: 'failed'})))
+            .toBeUndefined();
+    });
+
+    it('treats unparseable text as data', ()=>{
+        expect(snapshot_running_status(envelope(200, 'not json at all')))
+            .toBeUndefined();
+    });
+});

src/__tests__/utils/client.request.test.ts

@@ -0,0 +1,185 @@
+import {describe, it, expect, beforeEach, afterEach, vi} from 'vitest';
+
+// load_config() reads a real file from the user's config dir and can override
+// api_url, which would make URL assertions depend on the machine running the
+// suite. Pin it.
+vi.mock('../../utils/config', ()=>({
+    load: ()=>({api_url: 'https://api.brightdata.com'}),
+}));
+
+vi.mock('../../utils/output', ()=>({
+    dim: (s: string)=>s,
+}));
+
+import {
+    request,
+    get_with_status,
+    Client_api_error,
+} from '../../utils/client';
+
+const json_response = (status: number, body: unknown)=>new Response(
+    JSON.stringify(body),
+    {status, headers: {'content-type': 'application/json'}}
+);
+
+const text_response = (status: number, body: string)=>new Response(
+    body,
+    {status, headers: {'content-type': 'text/plain'}}
+);
+
+// Fast retries — real backoff starts at 500ms and doubles, which would add
+// seconds of wall-clock to the suite.
+const fast_retry = {retry: {base_ms: 1, max_ms: 2}};
+
+describe('utils/client.request', ()=>{
+    beforeEach(()=>{
+        vi.spyOn(console, 'error').mockImplementation(()=>{});
+    });
+
+    afterEach(()=>{
+        vi.unstubAllGlobals();
+        vi.restoreAllMocks();
+    });
+
+    it('returns the parsed body only (unchanged contract)', async()=>{
+        vi.stubGlobal('fetch', vi.fn(async()=>json_response(200, {ok: 1})));
+        await expect(request('key', '/x')).resolves.toEqual({ok: 1});
+    });
+
+    it('returns text when the response is not json', async()=>{
+        vi.stubGlobal('fetch', vi.fn(async()=>text_response(200, 'a,b\n')));
+        await expect(request('key', '/x')).resolves.toBe('a,b\n');
+    });
+
+    it('sends bearer auth to the resolved url', async()=>{
+        const fetch_mock = vi.fn(async()=>json_response(200, {}));
+        vi.stubGlobal('fetch', fetch_mock);
+        await request('secret', '/datasets/v3/snapshot/s1');
+        const [url, init] = fetch_mock.mock.calls[0] as unknown as
+            [string, RequestInit];
+        expect(url).toBe(
+            'https://api.brightdata.com/datasets/v3/snapshot/s1');
+        expect((init.headers as Record<string, string>)['Authorization'])
+            .toBe('Bearer secret');
+    });
+
+    describe('get_with_status', ()=>{
+        it('exposes the status code alongside the body', async()=>{
+            vi.stubGlobal('fetch',
+                vi.fn(async()=>json_response(200, {rows: 1})));
+            const env = await get_with_status('key', '/x');
+            expect(env.status).toBe(200);
+            expect(env.body).toEqual({rows: 1});
+        });
+
+        it('surfaces 202 rather than hiding it behind the body', async()=>{
+            // 202 is res.ok, so before this the caller could not tell an
+            // accepted-but-unfinished job from finished data.
+            vi.stubGlobal('fetch',
+                vi.fn(async()=>json_response(202, {status: 'running'})));
+            const env = await get_with_status('key', '/x');
+            expect(env.status).toBe(202);
+        });
+
+        it('exposes response headers', async()=>{
+            vi.stubGlobal('fetch',
+                vi.fn(async()=>json_response(200, {})));
+            const env = await get_with_status('key', '/x');
+            expect(env.headers.get('content-type'))
+                .toContain('application/json');
+        });
+    });
+
+    describe('error typing', ()=>{
+        it('throws a Client_api_error carrying the status', async()=>{
+            vi.stubGlobal('fetch',
+                vi.fn(async()=>text_response(404, 'no such dataset')));
+            await expect(request('key', '/x', fast_retry))
+                .rejects.toBeInstanceOf(Client_api_error);
+            try {
+                await request('key', '/x', fast_retry);
+            } catch(e) {
+                const err = e as Client_api_error;
+                expect(err.status).toBe(404);
+                // message bytes are part of the contract: scraper-studio
+                // matches on error prose (e.g. 'realtime job limit')
+                expect(err.message).toBe(
+                    'Error: no such dataset\n'
+                    +'  Status: 404\n'
+                    +'  Hint: Resource not found. Check the URL or dataset '
+                    +'type.'
+                );
+            }
+        });
+
+        it('does not retry an API error', async()=>{
+            const fetch_mock = vi.fn(async()=>text_response(400, 'bad input'));
+            vi.stubGlobal('fetch', fetch_mock);
+            await expect(request('key', '/x', fast_retry)).rejects.toThrow();
+            expect(fetch_mock).toHaveBeenCalledTimes(1);
+        });
+
+        it('retries a network error whose message starts with "Error:"',
+            async()=>{
+                // Retry used to be decided by message.startsWith('Error:'),
+                // so a network failure worded this way was misclassified as a
+                // final API error and never retried.
+                const fetch_mock = vi.fn(async()=>{
+                    throw new Error('Error: socket hang up');
+                });
+                vi.stubGlobal('fetch', fetch_mock);
+                await expect(request('key', '/x', fast_retry))
+                    .rejects.toThrow('Network request failed');
+                expect(fetch_mock).toHaveBeenCalledTimes(4);
+            });
+
+        it('retries transient statuses then surfaces the error', async()=>{
+            const fetch_mock = vi.fn(async()=>text_response(503, 'busy'));
+            vi.stubGlobal('fetch', fetch_mock);
+            await expect(request('key', '/x', fast_retry)).rejects.toThrow();
+            expect(fetch_mock).toHaveBeenCalledTimes(4);
+        });
+    });
+
+    describe('request timeout', ()=>{
+        // A hung connection produces no error at all, so without an abort the
+        // retry loop never engages and the CLI waits forever.
+        const hang_until_aborted = (_url: string, init: RequestInit)=>
+            new Promise<Response>((_resolve, reject)=>{
+                init.signal?.addEventListener('abort', ()=>{
+                    const err = new Error('aborted');
+                    err.name = 'TimeoutError';
+                    reject(err);
+                });
+            });
+
+        it('aborts a hung request instead of hanging forever', async()=>{
+            vi.stubGlobal('fetch', vi.fn(hang_until_aborted));
+            await expect(request('key', '/x', {
+                timeout_ms: 20,
+                ...fast_retry,
+            })).rejects.toThrow('Request timed out after 0s');
+        });
+
+        it('retries a timeout once, not the full retry budget', async()=>{
+            // timeout x attempts multiplies the user-visible stall, so the
+            // generic budget (3 retries) is deliberately not reused here.
+            const fetch_mock = vi.fn(hang_until_aborted);
+            vi.stubGlobal('fetch', fetch_mock);
+            await expect(request('key', '/x', {
+                timeout_ms: 20,
+                ...fast_retry,
+            })).rejects.toThrow();
+            expect(fetch_mock).toHaveBeenCalledTimes(2);
+        });
+
+        it('passes an abort signal on every attempt', async()=>{
+            const fetch_mock = vi.fn(async()=>json_response(200, {}));
+            vi.stubGlobal('fetch', fetch_mock);
+            await request('key', '/x');
+            const [, init] = fetch_mock.mock.calls[0] as unknown as
+                [string, RequestInit];
+            expect(init.signal).toBeInstanceOf(AbortSignal);
+        });
+    });
+});

src/commands/dataset.ts

@@ -1,6 +1,7 @@
 import {Command} from 'commander';
 import {ensure_authenticated} from '../utils/auth';
-import {get, post} from '../utils/client';
+import {get_with_status, post} from '../utils/client';
+import type {Response_envelope} from '../utils/client';
 import {print, dim, fail} from '../utils/output';
 import {start as start_spinner} from '../utils/spinner';
 import {parse_timeout, poll_until} from '../utils/polling';
@@ -214,6 +215,36 @@ const extract_status = (result: unknown): string|undefined=>{
     return undefined;
 };
 
+// A snapshot body only parses to an object when the requested format is json.
+// Under csv/ndjson/jsonl the client hands back text, so a status payload would
+// arrive as a string and extract_status would miss it.
+const parse_if_json_text = (body: unknown): unknown=>{
+    if (typeof body != 'string')
+        return body;
+    try {
+        return JSON.parse(body);
+    } catch(_e) {
+        return body;
+    }
+};
+
+// Is this snapshot still building? Three signals, most authoritative first:
+//   1. HTTP 202 — the server says "accepted, not done". Trusted outright.
+//   2. a parsed body carrying a running status.
+//   3. a *text* body that parses to one (the csv/ndjson/jsonl case above).
+// Returns the running status string (so progress output keeps showing
+// starting/building/running rather than a flattened literal), or undefined
+// when the response is the data.
+const snapshot_running_status = (
+    env: Response_envelope
+): string|undefined=>{
+    const status = extract_status(parse_if_json_text(env.body));
+    const is_running = !!status && RUNNING_STATUSES.includes(status);
+    if (env.status == 202)
+        return is_running ? status : 'building';
+    return is_running ? status : undefined;
+};
+
 const handle_pipelines = async(
     dataset_type_raw: string,
     params: string[],
@@ -266,14 +297,15 @@ const handle_pipelines = async(
         }
         console.error(dim(`Triggered collection with snapshot ID:` +
             `${snapshot_id}`));
-        const poll_result = await poll_until<unknown>({
+        const poll_result = await poll_until<Response_envelope>({
             timeout_seconds: timeout,
             fetch_once: ()=>{
                 const endpoint = `${SNAPSHOT_ENDPOINT}/${snapshot_id}`
                     +`?format=${format}`;
-                return get<unknown>(api_key, endpoint, {timing: opts.timing});
+                return get_with_status<unknown>(
+                    api_key, endpoint, {timing: opts.timing});
             },
-            get_status: extract_status,
+            get_status: snapshot_running_status,
             running_statuses: RUNNING_STATUSES,
             timeout_label: 'data',
             on_running: ({attempt, timeout_seconds, status})=>{
@@ -286,7 +318,7 @@ const handle_pipelines = async(
         console.error(dim(
             `Data received after ${poll_result.attempts} attempts`
         ));
-        const result = poll_result.result;
+        const result = poll_result.result.body;
         const cleaned_result = format == 'json' ? strip_nulls(result) : result;
         print(cleaned_result, {
             json: opts.json,
@@ -333,4 +365,4 @@ add_examples(pipelines_command, [
     },
 ]);
 
-export {pipelines_command, handle_pipelines};
+export {pipelines_command, handle_pipelines, snapshot_running_status};