TE-27460: add --sync to upload-pdf - #558
Conversation
upload-pdf returned as soon as the files were accepted, so results were only reachable by polling the build afterwards with --fetch-results. --sync mints a uuid per document, sends it with the upload, and waits for each document's comparison to finish before printing the results or writing them to the --fetch-results file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V4Hy9BKNiBTDThi87RYSqX
sushobhit-lt
left a comment
sushobhit-lt
left a comment
There was a problem hiding this comment.
Review of --sync. One blocker: the flag never reaches ctx.options, so none of the new code paths execute. Beyond that, the sync path as written is not safe to gate a CI pipeline with.
| break; | ||
| } | ||
| } catch (error: any) { | ||
| ctx.log.debug(`sync poll failed for ${target.name}: ${error.message}`); |
There was a problem hiding this comment.
Fatal HTTP errors are swallowed as "still processing", hanging the build for 10 minutes.
This catch treats every failure as a transient "not ready yet" and keeps polling. httpClient.request throws on any non-2xx, so a 401 from an expired/invalid project token, a 403, a 500, or a wrong buildId all land here, get a debug-level line the user won't see at default verbosity, and then the loop sleeps 5s and retries until the 10-minute deadline — after which the document is reported as snapshotStatus: 'processing', Timed out waiting for results. A misconfigured token turns into a 10-minute CI hang with a misleading message.
Mirror what server.ts already does for this endpoint: retry only on the pending statuses (202/404), and on anything else (auth failures, 5xx, unexpected shape) stop polling and surface the real error.
There was a problem hiding this comment.
Fixed in 10d3d4d. Only 202 and 404 are treated as "not ready" now. Anything else — 401, 403, 5xx, or an unexpected shape — stops polling immediately, records the document as failed with the real error message, and surfaces it at error level rather than debug.
Your point about the misleading message was the sharpest part: a bad token previously produced a ten minute wait followed by "Timed out waiting for results", which points the user at the wrong problem entirely.
…t should - ctx.options is an explicit whitelist, and sync was never added to it, so the flag was inert end to end: no uuids, no sync on the upload, and the command fell through to the old fetch-results path. - Sync exists to gate a pipeline, so a mismatch, a failed document or a timeout now exits non-zero instead of passing green. - Only 202 and 404 mean "not ready". Auth failures and 5xx were being retried silently until the deadline, turning a bad token into a ten minute hang with a misleading timeout message. They now stop polling and report the real error. - Each document gets its own deadline. On one shared budget a slow first document timed out the rest before they were ever requested. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V4Hy9BKNiBTDThi87RYSqX
Exiting non-zero on a mismatch was wrong. A mismatch is a result for the caller to review, not a CLI failure, and the web sync path does not fail the run for one either. Sync now always exits 0 and only prints or writes the results. The per-document deadline and the terminal-error handling from the same review pass are unchanged; a failed or timed out document is still reported in the output, it just no longer changes the exit code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V4Hy9BKNiBTDThi87RYSqX
A batch now takes as long as its slowest pdf instead of the sum of all of them, which matches how web behaves: the SDK calls the same endpoint once per snapshot, so several of its polls are already in flight at once. Each document resolves rather than rejects, so one failed pdf cannot discard the results of the others. Concurrency is capped so a large upload does not put one in-flight request per document on every tick, and results are returned in upload order regardless of which finished first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V4Hy9BKNiBTDThi87RYSqX
Waiting for the whole batch meant a single slow pdf held back nine finished ones for up to the full timeout. Documents are still polled concurrently, but each is reported as soon as it and everything before it is ready, so results appear in the order the files were sent. The results file is rewritten as each document lands, matching how the non-sync pdf poller already writes it. No summary is emitted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V4Hy9BKNiBTDThi87RYSqX
Covers the --sync flag on upload-pdf, merged in LambdaTest#558 without a bump, and the 404 handling fix in this PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V4Hy9BKNiBTDThi87RYSqX
2 participants
upload-pdf returned as soon as the files were accepted, so results were only reachable by polling the build afterwards with --fetch-results.
--sync mints a uuid per document, sends it with the upload, and waits for each document's comparison to finish before printing the results or writing them to the --fetch-results file.
Claude-Session: https://claude.ai/code/session_01V4Hy9BKNiBTDThi87RYSqX