improve shx upload error handling

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

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 <path>` 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.

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
 }
 

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)"