Skip to content

Commit 8ff84ae

Browse files
fix(trace-recent): structured :bad-arg on non-integer window, no Java stack
Closes #16. trace-recent-op called Integer/parseInt on the first positional arg outside any try/catch. Passing a stray flag (e.g. trace-recent.sh --limit 25) leaked a NumberFormatException stack to stderr — violating the skill's "every script returns structured EDN" contract and giving agents nothing translatable to the user. Now: parse is wrapped; on failure, dies with {:reason :bad-arg :got <input> :hint "..."} explaining that the window is in milliseconds (not a count, the natural mistake) and showing the correct usage. Adds a :bad-arg row to docs/skill/troubleshooting.md so agents have a known mapping. 2 new deftests cover both branches: non-integer arg surfaces :bad-arg via die; valid integer proceeds past parse without dying. Note: 5 sibling parse sites have the same Integer/Long parseInt-without- try shape (watch-op flags --window-ms / --count / --idle-ms / --hard-ms / --poll-ms; tail-build's --wait-ms; the predicate parser's --timing-ms already handles bad input via parse-predicate-args). A sweep fixing those is worth filing as a follow-up but out of scope for this bug which was specifically against trace-recent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e953b5c commit 8ff84ae

3 files changed

Lines changed: 46 additions & 1 deletion

File tree

docs/skill/troubleshooting.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ Every script returns structured edn like `{:ok? false :reason ...}`. Translate t
1515
| `:handler-error` | The user's handler threw. Point at `:handler/error` and stack details. |
1616
| `:timed-out? true` | The dispatch cascade did not settle. The tab may be backgrounded, debugger paused, or async chain continuing. |
1717
| `:connection :lost` | Reconnect with `scripts/discover-app.sh`. |
18+
| `:bad-arg` | A positional or flag arg failed to parse. The response carries `:got` (the offending input) and `:hint` (correct usage). Common cause: passing a flag where a positional integer is expected — e.g. `trace-recent.sh --limit 25` (the script takes `<window-ms>` as its only positional). |
1819

1920
## Multi-Build Setups
2021

scripts/ops.clj

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1386,7 +1386,15 @@
13861386
(defn- trace-recent-op [args]
13871387
(ensure-port!)
13881388
(when (empty? args) (die :missing-window :hint "usage: trace-recent <ms>"))
1389-
(let [ms (Integer/parseInt (first args))
1389+
(let [raw-ms (first args)
1390+
ms (try (Integer/parseInt raw-ms)
1391+
(catch NumberFormatException _
1392+
(die :bad-arg
1393+
:got raw-ms
1394+
:hint (str "trace-recent expects <window-ms> as a positive integer "
1395+
"(milliseconds, NOT epoch count). "
1396+
"Got non-integer: " (pr-str raw-ms) ". "
1397+
"Usage: trace-recent.sh <window-ms> [--build=<id>]"))))
13901398
build-id (build-id-from-args (rest args))
13911399
reinjected? (ensure-injected! build-id)]
13921400
(try

tests/ops_smoke.bb

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1260,6 +1260,42 @@
12601260
(is (str/includes? (:hint @emitted) "register-epoch-cb")
12611261
":hint mentions the native cb as an alternative to 10x")))))
12621262

1263+
;; ---------------------------------------------------------------------------
1264+
;; Tests for issue #16: trace-recent.sh must surface parse failures as
1265+
;; structured EDN, not leak Java stack traces.
1266+
;; ---------------------------------------------------------------------------
1267+
1268+
(deftest trace-recent-non-integer-window-surfaces-bad-arg
1269+
(testing "trace-recent with a non-integer window emits :reason :bad-arg via die, not a NumberFormatException"
1270+
(let [die-args (atom nil)]
1271+
(with-redefs [ops/ensure-port! (fn [] true)
1272+
;; die is auto-throw-mocked by the :each fixture for safety;
1273+
;; this test additionally captures the args.
1274+
ops/die (fn [reason & {:as extra}]
1275+
(reset! die-args (assoc extra :reason reason))
1276+
(throw (ex-info "die" {:reason reason})))]
1277+
(try (#'ops/trace-recent-op ["--limit"])
1278+
(catch clojure.lang.ExceptionInfo _))
1279+
(is (= :bad-arg (:reason @die-args))
1280+
"structured :reason instead of bare NumberFormatException")
1281+
(is (= "--limit" (:got @die-args))
1282+
":got carries the offending input verbatim")
1283+
(is (str/includes? (:hint @die-args) "milliseconds")
1284+
":hint clarifies that the window is in ms, not a count (the natural mistake)")))))
1285+
1286+
(deftest trace-recent-valid-integer-window-passes-through
1287+
(testing "trace-recent with a valid integer window proceeds past parse without die-ing"
1288+
(let [die-called? (atom false)]
1289+
(with-redefs [ops/ensure-port! (fn [] true)
1290+
ops/die (fn [& _]
1291+
(reset! die-called? true)
1292+
(throw (ex-info "should-not-die" {})))
1293+
ops/ensure-injected! (fn [_] false)
1294+
ops/cljs-eval-value (fn [& _] [])
1295+
ops/emit (fn [_] nil)]
1296+
(#'ops/trace-recent-op ["3000"])
1297+
(is (false? @die-called?) "valid integer arg does not trigger any die")))))
1298+
12631299
;; ---------------------------------------------------------------------------
12641300
;; Tests for issue #14: refuse non-shadow-cljs nREPLs early with a structured
12651301
;; :unsupported-build-tool, instead of crashing on the first call to

0 commit comments

Comments
 (0)