From 17bd6622eb36e9bb3187b91df9d4a31a6983a5b9 Mon Sep 17 00:00:00 2001 From: Luna Date: Thu, 4 Jun 2026 22:10:55 +0000 Subject: [PATCH] improve shx upload error handling --- README.md | 4 +- TODO.md | 23 ++-- bin/shx-upload | 70 +++++++++++- tests/smoke.sh | 300 +++++++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 353 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 9ecf23d..456bea0 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ tiny helper repo for uploads to the SHX Zipline instance. - `bin/shx-upload` - upload one or more files to Zipline - `.gitea/workflows/ci.yml` - basic CI for syntax + smoke tests - `.gitea/workflows/release.yml` - tag build artifact packaging -- `tests/smoke.sh` - local smoke test with mocked network calls +- `tests/smoke.sh` - local smoke tests covering success, failures, and request formation with mocked network calls ## requirements @@ -50,6 +50,8 @@ push a tag like `v0.1.0` to trigger the release packaging workflow. - username/password are for dashboard login only - actual uploads use a Zipline API token +- failed uploads now print the Zipline response body when one is returned +- successful uploads are validated to ensure the response includes a non-empty `.files` array - docs: https://zipline.diced.sh/docs - upload endpoint ref: https://v3.zipline.diced.sh/docs/api/upload - shell uploader guide: https://zipline.diced.sh/docs/guides/uploaders/shell-script diff --git a/TODO.md b/TODO.md index e1a46a0..9367155 100644 --- a/TODO.md +++ b/TODO.md @@ -2,13 +2,11 @@ ## high value -- [ ] surface Zipline error bodies on failed uploads - - right now `curl -fsS` drops the JSON body on HTTP 4xx/5xx, which makes auth/quota/debugging annoyingly blind. - - keep non-zero exits, but capture the response body and print the server message when available. +- [x] surface Zipline error bodies on failed uploads + - HTTP failures now preserve the response body and print a parsed server message when available. -- [ ] validate the response shape before printing success output - - the script currently assumes the API returns `.files`; if Zipline returns a different success/error shape, users can get empty output with little context. - - fail clearly when `.files` is missing or empty. +- [x] validate the response shape before printing success output + - successful responses are now rejected unless they include a non-empty `.files` array. - [ ] add a `--no-env` / `--env-file ` flag - auto-loading `/root/.openclaw/.env` is handy here, but it makes the helper less portable and harder to use in CI or on another machine. @@ -16,13 +14,8 @@ ## reliability / test gaps -- [ ] add tests for the unhappy paths that matter most - - missing file - - missing token - - missing `jq` in normal mode - - server-side upload failure - - this script is tiny, so covering the sharp edges will buy more confidence than adding features. +- [x] add tests for the unhappy paths that matter most + - covered: missing file, missing token, missing `jq` in normal mode, server-side upload failure, and malformed success payloads. -- [ ] add a mock assertion that the upload request is formed correctly - - current smoke test only checks the printed output. - - also assert that `curl` was called with the auth header and multipart `file=@...` field so refactors do not silently break real uploads. +- [x] add a mock assertion that the upload request is formed correctly + - the smoke tests now assert the auth header, upload endpoint, and multipart `file=@...` field. diff --git a/bin/shx-upload b/bin/shx-upload index eff395c..ef22b82 100755 --- a/bin/shx-upload +++ b/bin/shx-upload @@ -24,6 +24,44 @@ Notes: EOF } +have_jq() { + command -v jq >/dev/null 2>&1 +} + +print_server_message() { + local response="$1" + local message="" + + [[ -n "$response" ]] || return 0 + + if have_jq; then + message="$( + printf '%s\n' "$response" | + jq -r 'if type=="object" then .error // .message // .reason // empty else empty end' 2>/dev/null || + true + )" + fi + + if [[ -n "$message" ]]; then + echo "server: $message" >&2 + else + echo "server response: $response" >&2 + fi +} + +response_has_files() { + local response="$1" + + if have_jq; then + printf '%s\n' "$response" | jq -e '.files | arrays and length > 0' >/dev/null 2>&1 + return $? + fi + + [[ "$response" == *'"files"'* ]] || return 1 + [[ "$response" =~ \"files\"[[:space:]]*:[[:space:]]*\[[[:space:]]*\] ]] && return 1 + return 0 +} + want_json=0 if [[ "${1:-}" == "--help" || "${1:-}" == "-h" ]]; then usage @@ -60,25 +98,47 @@ fi upload_one() { local file="$1" + local body_file http_code response if [[ ! -f "$file" ]]; then echo "error: file not found: $file" >&2 return 1 fi - local response - response="$(curl -fsS \ + body_file="$(mktemp)" + if ! http_code="$(curl -sS \ + -o "$body_file" \ + -w '%{http_code}' \ -H "authorization: $SHX_TOKEN" \ "$SHX_URL/api/upload" \ - -F "file=@$file")" + -F "file=@$file")"; then + rm -f "$body_file" + echo "error: upload request failed for $file" >&2 + return 1 + fi + + response="$(cat "$body_file")" + rm -f "$body_file" + + if [[ ! "$http_code" =~ ^2[0-9][0-9]$ ]]; then + echo "error: upload failed for $file (HTTP $http_code)" >&2 + print_server_message "$response" + return 1 + fi + + if ! response_has_files "$response"; then + echo "error: upload response missing a non-empty .files array for $file" >&2 + print_server_message "$response" + return 1 + fi if [[ $want_json -eq 1 ]]; then printf '%s\n' "$response" else - if ! command -v jq >/dev/null 2>&1; then + if ! have_jq; then echo "error: jq is required unless --json is used" >&2 return 1 fi - printf '%s\n' "$response" | jq -r 'if .files then .files[] else . end' + printf '%s\n' "$response" | jq -r '.files[]' fi } diff --git a/tests/smoke.sh b/tests/smoke.sh index 785bff0..a8bf6b5 100755 --- a/tests/smoke.sh +++ b/tests/smoke.sh @@ -2,37 +2,291 @@ set -euo pipefail repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -workdir="$(mktemp -d)" -trap 'rm -rf "$workdir"' EXIT +tests_run=0 -cat >"$workdir/.env" <<'EOF' +pass() { + tests_run=$((tests_run + 1)) +} + +assert_eq() { + local expected="$1" + local actual="$2" + if [[ "$expected" != "$actual" ]]; then + echo "assertion failed: expected [$expected], got [$actual]" >&2 + exit 1 + fi +} + +assert_file_contains() { + local needle="$1" + local path="$2" + if ! grep -Fq "$needle" "$path"; then + echo "assertion failed: [$needle] not found in $path" >&2 + exit 1 + fi +} + +assert_file_not_contains() { + local needle="$1" + local path="$2" + if grep -Fq "$needle" "$path"; then + echo "assertion failed: [$needle] unexpectedly found in $path" >&2 + exit 1 + fi +} + +make_workdir() { + mktemp -d +} + +install_mock_curl() { + local workdir="$1" + cat >"$workdir/bin/curl" <<'EOF' +#!/usr/bin/env bash +set -euo pipefail + +printf '%s\n' "$@" >"$MOCK_CURL_ARGS_FILE" + +outfile="" +http_code="${MOCK_CURL_HTTP_CODE:-200}" +body="${MOCK_CURL_BODY:-}" + +if [[ -z "$body" ]]; then + body='{"files":["https://shx.example/mock-file"]}' +fi + +while [[ $# -gt 0 ]]; do + case "$1" in + -o) + outfile="$2" + shift 2 + ;; + -w) + shift 2 + ;; + *) + shift + ;; + esac +done + +if [[ "${MOCK_CURL_EXIT_CODE:-0}" != "0" ]]; then + echo "${MOCK_CURL_STDERR:-mock curl transport error}" >&2 + exit "$MOCK_CURL_EXIT_CODE" +fi + +printf '%s' "$body" >"$outfile" +printf '%s' "$http_code" +EOF + chmod +x "$workdir/bin/curl" +} + +install_mock_jq() { + local workdir="$1" + cat >"$workdir/bin/jq" <<'EOF' +#!/usr/bin/env python3 +import json +import sys + +args = sys.argv[1:] +raw = sys.stdin.read() +payload = json.loads(raw) + +flags = [arg for arg in args if arg.startswith("-")] +query = next((arg for arg in args if not arg.startswith("-")), "") + +if query == '.files | arrays and length > 0': + files = payload.get("files") + ok = isinstance(files, list) and len(files) > 0 + sys.exit(0 if ok else 1) + +if query == '.files[]': + files = payload.get("files") + if not isinstance(files, list): + sys.exit(1) + for item in files: + print(item) + sys.exit(0) + +if query == 'if type=="object" then .error // .message // .reason // empty else empty end': + if isinstance(payload, dict): + for key in ("error", "message", "reason"): + value = payload.get(key) + if value: + print(value) + sys.exit(0) + sys.exit(0) + +raise SystemExit(f"unsupported jq query: {query} with flags {flags}") +EOF + chmod +x "$workdir/bin/jq" +} + +write_env() { + local workdir="$1" + cat >"$workdir/.env" <<'EOF' SHX_URL=https://shx.example SHX_TOKEN=test-token EOF +} -mkdir -p "$workdir/bin" -cat >"$workdir/bin/curl" <<'EOF' -#!/usr/bin/env bash -set -euo pipefail -printf '%s\n' '{"files":["https://shx.example/mock-file"]}' +run_upload() { + local workdir="$1" + shift + ( + cd "$repo_root" + env -u SHX_TOKEN -u ZIPLINE_TOKEN \ + PATH="$workdir/bin:${PATH:-}" \ + SHX_ENV_FILE="$workdir/.env" \ + MOCK_CURL_ARGS_FILE="$workdir/curl-args.txt" \ + "$repo_root/bin/shx-upload" "$@" + ) +} + +test_success_and_request_shape() { + local workdir + workdir="$(make_workdir)" + + mkdir -p "$workdir/bin" + write_env "$workdir" + install_mock_curl "$workdir" + install_mock_jq "$workdir" + touch "$workdir/demo.txt" + + run_upload "$workdir" "$workdir/demo.txt" >"$workdir/out.txt" 2>"$workdir/err.txt" + + assert_file_contains 'https://shx.example/mock-file' "$workdir/out.txt" + assert_file_contains 'authorization: test-token' "$workdir/curl-args.txt" + assert_file_contains "file=@$workdir/demo.txt" "$workdir/curl-args.txt" + assert_file_contains '/api/upload' "$workdir/curl-args.txt" + assert_file_not_contains 'error:' "$workdir/err.txt" + rm -rf "$workdir" + pass +} + +test_missing_file() { + local workdir + workdir="$(make_workdir)" + + mkdir -p "$workdir/bin" + write_env "$workdir" + install_mock_curl "$workdir" + + if run_upload "$workdir" "$workdir/does-not-exist.txt" >"$workdir/out.txt" 2>"$workdir/err.txt"; then + echo "expected missing file failure" >&2 + exit 1 + fi + + assert_file_contains 'error: file not found:' "$workdir/err.txt" + rm -rf "$workdir" + pass +} + +test_missing_token() { + local workdir + workdir="$(make_workdir)" + + mkdir -p "$workdir/bin" + cat >"$workdir/.env" <<'EOF' +SHX_URL=https://shx.example EOF -chmod +x "$workdir/bin/curl" + install_mock_curl "$workdir" + touch "$workdir/demo.txt" -cat >"$workdir/bin/jq" <<'EOF' -#!/usr/bin/env python3 -import json, sys -payload = json.load(sys.stdin) -for item in payload.get("files", []): - print(item) -EOF -chmod +x "$workdir/bin/jq" + if run_upload "$workdir" "$workdir/demo.txt" >"$workdir/out.txt" 2>"$workdir/err.txt"; then + echo "expected missing token failure" >&2 + exit 1 + fi -touch "$workdir/demo.txt" + assert_file_contains 'error: SHX_TOKEN is not set' "$workdir/err.txt" + rm -rf "$workdir" + pass +} -PATH="$workdir/bin:$PATH" \ -SHX_ENV_FILE="$workdir/.env" \ -"$repo_root/bin/shx-upload" "$workdir/demo.txt" >"$workdir/out.txt" +test_missing_jq_in_normal_mode() { + local workdir + workdir="$(make_workdir)" -grep -Fx 'https://shx.example/mock-file' "$workdir/out.txt" + mkdir -p "$workdir/bin" + write_env "$workdir" + install_mock_curl "$workdir" + ln -s /bin/bash "$workdir/bin/bash" + ln -s /bin/cat "$workdir/bin/cat" + ln -s /bin/rm "$workdir/bin/rm" + ln -s /usr/bin/mktemp "$workdir/bin/mktemp" + touch "$workdir/demo.txt" -echo 'smoke ok' + ( + cd "$repo_root" + env -u SHX_TOKEN -u ZIPLINE_TOKEN \ + PATH="$workdir/bin" \ + SHX_ENV_FILE="$workdir/.env" \ + MOCK_CURL_ARGS_FILE="$workdir/curl-args.txt" \ + /bin/bash "$repo_root/bin/shx-upload" "$workdir/demo.txt" + ) >"$workdir/out.txt" 2>"$workdir/err.txt" && { + echo "expected missing jq failure" >&2 + exit 1 + } + + assert_file_contains 'error: jq is required unless --json is used' "$workdir/err.txt" + rm -rf "$workdir" + pass +} + +test_server_failure_surfaces_body() { + local workdir + workdir="$(make_workdir)" + + mkdir -p "$workdir/bin" + write_env "$workdir" + install_mock_curl "$workdir" + install_mock_jq "$workdir" + touch "$workdir/demo.txt" + + ( + export MOCK_CURL_HTTP_CODE=401 + export MOCK_CURL_BODY='{"error":"invalid token"}' + run_upload "$workdir" "$workdir/demo.txt" + ) >"$workdir/out.txt" 2>"$workdir/err.txt" && { + echo "expected server failure" >&2 + exit 1 + } + + assert_file_contains 'error: upload failed for' "$workdir/err.txt" + assert_file_contains 'server: invalid token' "$workdir/err.txt" + rm -rf "$workdir" + pass +} + +test_invalid_success_shape_fails() { + local workdir + workdir="$(make_workdir)" + + mkdir -p "$workdir/bin" + write_env "$workdir" + install_mock_curl "$workdir" + install_mock_jq "$workdir" + touch "$workdir/demo.txt" + + ( + export MOCK_CURL_BODY='{"success":true}' + run_upload "$workdir" "$workdir/demo.txt" + ) >"$workdir/out.txt" 2>"$workdir/err.txt" && { + echo "expected invalid response failure" >&2 + exit 1 + } + + assert_file_contains 'error: upload response missing a non-empty .files array' "$workdir/err.txt" + assert_file_contains 'server response: {"success":true}' "$workdir/err.txt" + rm -rf "$workdir" + pass +} + +test_success_and_request_shape +test_missing_file +test_missing_token +test_missing_jq_in_normal_mode +test_server_failure_surfaces_body +test_invalid_success_shape_fails + +echo "smoke ok ($tests_run tests)"