fix: ScanLink worker and request stress failures (0.8.0b1)

CHANGELOG.md

@@ -2,6 +2,15 @@
 
 This file owns the release history for `ScanLink`.
 
+## v0.8.0b1
+
+- **Atomic PDF Completion**: Keep child completion provisional until the process exits and the parent stores result bytes; publish terminal errors with their details.
+- **Parent Scan Deadline**: Enforce the configured scan deadline even when the child keeps reporting progress, independently of GUI and idle limits.
+- **Scan Payload Validation**: Reject non-object JSON, invalid metadata/callback types and non-JSON protocol requests before queueing hardware work.
+- **Single-Use Result Metadata**: Free consumed PDF bytes immediately while retaining job metadata until its TTL so racing downloads consistently receive 410.
+- **One Scan POST Per Attempt**: Negotiate transport through health checks and report `scan_request_uncertain` after a lost scan response instead of resubmitting over another transport.
+- **Real Worker Regressions**: Exercise actual child-process completion, parent timeout enforcement, malformed requests and competing PDF downloads with only the scanner child simulated.
+
 ## v0.7.5b1
 
 - **Windows x86 Crypto Wheels**: Resolve cryptography from compatible binary wheels during executable builds, avoiding an unsupported latest-source Rust/OpenSSL compilation on 32-bit Python.

VERSION

@@ -1 +1 @@
-0.7.5b1
+0.8.0b1

docs/api.md

@@ -84,6 +84,8 @@ Errors:
 | HTTP | Code | Meaning |
 | --- | --- | --- |
 | 400 | `invalid_json` | Body could not be parsed as JSON. |
+| 400 | `invalid_payload` | JSON root was not an object. |
+| 400 | `invalid_callback` | Callback URL was not a string or could not be parsed. |
 | 400 | `invalid_metadata` | `metadata` was provided but is not an object. |
 | 409 | `busy` | Another scan job is queued or running. |
 | 415 | `json_required` | Request did not use JSON. |
@@ -144,7 +146,7 @@ Errors:
 
 ## GET /scan/<job_id>/result
 
-Downloads the scanned PDF for a completed job.
+Downloads the scanned PDF for a completed job. `completed` is published only after the child has exited and the parent has stored the PDF. Concurrent downloads are single-use: exactly one receives the bytes; later requests return `410` until the job metadata expires. PDF bytes are freed immediately after retrieval, while metadata remains until `job_ttl_seconds` from completion.
 
 Response:
 
@@ -158,10 +160,12 @@ Errors:
 | --- | --- | --- |
 | 404 | `job_not_found` | Job does not exist or expired. |
 | 409 | `result_not_ready` | Job is not completed yet. |
-| 410 | `result_unavailable` | Result was already consumed or expired. |
+| 410 | `result_unavailable` | Result was already consumed. |
 
 ## POST /internal/protocol/scan
 
+Requires an `application/json` object and applies the same metadata/callback validation as `/scan`; malformed requests never queue a scan. The optional `action` must be `scan`.
+
 Internal endpoint used by `scanlink://` launches to forward commands into an already-running ScanLink instance. Application integrations should normally use `POST /scan`.
 
 ## POST /shutdown
@@ -215,3 +219,11 @@ runs, `ready` after trusted HTTPS starts, and `failed` after a setup/listener
 failure. Existing `http_enabled` and `https_enabled` remain listener flags.
 Older clients may ignore the new field; newer clients must tolerate its absence
 on older helpers. Existing HTTP scan jobs remain usable while setup runs.
+
+## Browser transport retries
+
+The bundled JavaScript client negotiates a healthy transport before its first scan.
+A scan POST is sent once on that transport. If the response or its body is lost,
+it rejects with `scan_request_uncertain` instead of resending the request over
+the other transport; the helper may already be scanning. Check ScanLink before
+starting another scan. Health and status reads retain transport fallback.

docs/architecture.md

@@ -204,3 +204,16 @@ When launched with a `scanlink://` URL:
 4. If no instance is running, the new process becomes the tray instance and queues the protocol scan after startup.
 
 This prevents duplicate tray apps and competing scanner sessions.
+
+## Worker completion and deadlines
+
+Child progress is provisional. A child `completed` update remains
+`generating_pdf` to API clients until the process exits and the parent stores the
+PDF atomically with the final `completed` status. Child failure states are also
+published by the parent with their error details after process exit.
+
+The parent enforces both `scan_timeout_seconds` and `gui_task_timeout_seconds`;
+the first enabled deadline terminates the child, even if it keeps reporting
+progress. Idle timeout remains an independent progress guard. Downloading frees
+the PDF immediately and retains consumed-job metadata until the normal job TTL,
+so repeated or racing downloads report `410` while that metadata exists.

docs/developer-guide.md

@@ -215,3 +215,12 @@ HTTP remains available during setup; clients should not block working HTTP
 scans while waiting for HTTPS. A native Retry dialog and tray repair action
 handle failed setup without a command prompt. See the architecture guide for
 DPAPI-protected keys, trust verification, hourly checks and renewal.
+
+### Lost scan responses
+
+The JavaScript client health-checks a transport before scanning and never
+automatically retries a scan POST. Handle `scan_request_uncertain` by asking the
+operator to check the tray app before starting another scan. A lost response can
+mean the request was accepted; retrying automatically could trigger another
+scanner dialog. Both public and protocol scan endpoints require a JSON object
+and validate metadata/callback types before queueing hardware work.

server.py

@@ -3,6 +3,7 @@
 import ssl
 import threading
 import time
+from urllib.parse import urlparse
 
 from flask import Flask, Response, jsonify, request, send_file
 from werkzeug.serving import make_server
@@ -56,6 +57,42 @@ def error_response(status_code, code, message, **extra):
         response.status_code = status_code
         return apply_cors(response)
 
+    def scan_payload():
+        if not request.is_json:
+            return None, error_response(
+                415, "json_required", "Scan requests must use an application/json body."
+            )
+        data = request.get_json(silent=True)
+        if data is None:
+            return None, error_response(
+                400, "invalid_json", "Request body could not be parsed as JSON."
+            )
+        if not isinstance(data, dict):
+            return None, error_response(
+                400, "invalid_payload", "Scan requests must contain a JSON object."
+            )
+        if data.get("metadata") is not None and not isinstance(data["metadata"], dict):
+            return None, error_response(
+                400, "invalid_metadata", "metadata must be a JSON object when provided."
+            )
+        for name in ("callback_url", "callback"):
+            if data.get(name) is not None and not isinstance(data[name], str):
+                return None, error_response(
+                    400,
+                    "invalid_callback",
+                    "callback_url must be a string when provided.",
+                )
+            if isinstance(data.get(name), str):
+                try:
+                    urlparse(data[name])
+                except ValueError:
+                    return None, error_response(
+                        400,
+                        "invalid_callback",
+                        "callback_url could not be parsed as a URL.",
+                    )
+        return data, None
+
     @app.after_request
     def add_cors_headers(response):
         if is_browser_endpoint(request.path):
@@ -88,22 +125,9 @@ def start_scan():
         if request.method == "OPTIONS":
             return empty_options_response()
 
-        if not request.is_json:
-            return error_response(
-                415, "json_required", "Scan requests must use an application/json body."
-            )
-
-        data = request.get_json(silent=True)
-        if data is None:
-            return error_response(
-                400, "invalid_json", "Request body could not be parsed as JSON."
-            )
-
-        metadata = data.get("metadata")
-        if metadata is not None and not isinstance(metadata, dict):
-            return error_response(
-                400, "invalid_metadata", "metadata must be a JSON object when provided."
-            )
+        data, error = scan_payload()
+        if error is not None:
+            return error
 
         doc_id = data.get("doc_id") or data.get("docId")
         callback_url = data.get("callback_url") or data.get("callback")
@@ -112,7 +136,7 @@ def start_scan():
             job = worker.submit_job(
                 doc_id=doc_id,
                 callback_url=callback_url,
-                metadata=metadata,
+                metadata=data.get("metadata"),
                 source="api",
             )
         except BusyError as exc:
@@ -176,7 +200,9 @@ def get_scan_result(job_id):
 
     @app.route("/internal/protocol/scan", methods=["POST"])
     def internal_protocol_scan():
-        data = request.get_json(silent=True) or {}
+        data, error = scan_payload()
+        if error is not None:
+            return error
         if data.get("action", "scan") != "scan":
             return (
                 jsonify(

static/scanlink.js

@@ -86,6 +86,9 @@ class ScanLink {
         if (options.metadata && typeof options.metadata === "object") payload.metadata = options.metadata;
 
         try {
+            if (!this.preferredUrl && !await this.checkHealth()) {
+                throw this.connectionError;
+            }
             const response = await this._fetchResponse("/scan", {
                 method: "POST",
                 headers: { "Content-Type": "application/json" },
@@ -97,7 +100,13 @@ class ScanLink {
                 throw await this._responseToError(response, "Failed to start scan.");
             }
 
-            const data = await response.json();
+            let data;
+            try {
+                data = await response.json();
+                if (!data || !data.job_id) throw new Error("Missing scan job ID");
+            } catch (error) {
+                throw this._uncertainRequest(error);
+            }
             this.currentJobId = data.job_id;
             this.onStatus(data);
             return data;
@@ -215,8 +224,14 @@ class ScanLink {
         window.location.href = url;
     }
 
+    _uncertainRequest(cause) {
+        return new ScanLinkError("The scan request lost its connection. Check ScanLink before starting another scan.", "scan_request_uncertain", { cause });
+    }
+
     async _fetchResponse(path, options = {}) {
-        const candidates = this._candidateUrls(false);
+        const readOnly = ["GET", "HEAD"].includes((options.method || "GET").toUpperCase());
+        const urls = this._candidateUrls(false);
+        const candidates = readOnly ? urls : urls.slice(0, 1);
         let lastError = null;
 
         for (const baseUrl of candidates) {
@@ -231,7 +246,7 @@ class ScanLink {
         }
 
         this.onConnectionChange(false);
-        this.connectionError = this._connectionFailure(lastError);
+        this.connectionError = readOnly ? this._connectionFailure(lastError) : this._uncertainRequest(lastError);
         throw this.connectionError;
     }
 

tests/fixtures/scan_child.py

@@ -0,0 +1,26 @@
+"""Hardware boundary fixture: the parent worker, process lifecycle and API stay real."""
+
+import json
+from pathlib import Path
+import sys
+import time
+
+directory = Path(sys.argv[1])
+scenario = sys.argv[2]
+
+
+def state(status, **extra):
+    pending = directory / "state.tmp"
+    pending.write_text(json.dumps({"status": status, **extra}))
+    pending.replace(directory / "state.json")
+
+
+state("acquiring", page_count=0)
+if scenario == "heartbeat":
+    for count in range(100):
+        state("page_scanned", page_count=count)
+        time.sleep(0.1)
+else:
+    (directory / "result.pdf").write_bytes(b"%PDF-1.4\nregression\n")
+    state("completed", page_count=1)
+    time.sleep(0.6)