Skip to content

Commit c881a41

Browse files
fix(eval): cljs-eval-wire throws :cljs-eval-empty on blank-with-no-error response
Closes #6 (defensive layer; root-cause investigation still requires the live fixture). The bug: \`scripts/eval-cljs.sh '(+ 1 2)'\` returned {:ok? true, :value nil, :rfp.wire/value-fits? true, :reinjected? true}. The {:ok? true} field is a false trust signal — there is no error, but the actual return value is silently dropped. Root cause path: shadow-cljs's nREPL response carries a blank :value (no :err, no :ex), parse-cljs-eval-response correctly returns nil for blank, cljs-eval-wire takes the trivial-passthrough branch and synthesizes {:value nil :rfp.wire/value-fits? true}, eval-op emits {:ok? true :value nil}. Each layer is locally correct; the stack loses the distinction between "form returned nil" and "shadow gave us no value at all." Fix: in cljs-eval-wire, when parse returns nil AND the underlying response has blank :value AND no :err AND no :ex, throw a structured ex-info {:reason :cljs-eval-empty :raw-response <res> :hint ...}. eval-op's existing try/catch surfaces it as {:ok? false :reason :cljs-eval-empty :data {:raw-response ...}}. Why surgical (in cljs-eval-wire, not parse-cljs-eval-response): the sentinel probe (\`runtime-already-injected?\`) and other internal callers legitimately treat nil-from-blank as "not present" — changing parse-cljs-eval-response would ripple through every cljs-eval-value call site. Legitimate-nil shape (e.g. \`(prn :hi)\`) is unaffected — shadow encodes that as :value "nil" (the EDN string), which parses to nil without tripping the blank check. 4 deftests cover: blank-no-error → :cljs-eval-empty; legitimate :value "nil" → passes through; blank with :ex set → existing :eval-error path; blank with nil :err and nil :ex (the headline #6 shape) → :cljs-eval-empty. Note: this closes the false-trust gap and surfaces \`:raw-response\` for debugging, but does not identify *why* shadow returns blank for the affected forms. Likely candidates flagged in the original report (shadow 2.20.11 result-shape change; wire/return! per-shape behavior) need verification against a live fixture and may be follow-ups. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0c09b67 commit c881a41

2 files changed

Lines changed: 56 additions & 0 deletions

File tree

scripts/ops.clj

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,20 @@
426426
([conn build-id form-str opts]
427427
(let [res (cljs-eval conn build-id form-str opts)
428428
parsed (parse-cljs-eval-response res)]
429+
;; Distinguish "form returned nil" from "shadow returned an
430+
;; empty response with no error". A legitimate nil result is
431+
;; encoded as :value "nil" (the EDN string), not as a blank
432+
;; :value. Blank :value with no :err and no :ex is a transport-
433+
;; or wrap-level failure that previously surfaced as the
434+
;; misleading {:ok? true :value nil} (issue #6).
435+
(when (and (nil? parsed)
436+
(str/blank? (str (:value res)))
437+
(str/blank? (str (:err res)))
438+
(nil? (:ex res)))
439+
(throw (ex-info "shadow-cljs returned an empty cljs-eval response"
440+
{:reason :cljs-eval-empty
441+
:raw-response res
442+
:hint "shadow-cljs's nREPL response carried no :value, :err, or :ex — neither a returned value nor an error. Common causes: a shadow-cljs version returning a value shape parse-cljs-eval-response doesn't recognize; the wire/return! wrapper evaluated but produced no transport output; the runtime ns is partially loaded (sentinel may say present but wire fns aren't bound). Re-run scripts/discover-app.sh to confirm runtime health."})))
429443
(if (wire-shape? parsed)
430444
(cond-> {:value (:rfp.wire/value parsed)
431445
:rfp.wire/cursor (:rfp.wire/cursor parsed)

tests/ops_smoke.bb

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,6 +1023,48 @@
10231023
(is (= {:counter 42} (#'ops/cljs-eval-value :app "(any-form)"))
10241024
"wire shape unwrapped; existing callers see the inner value")))))
10251025

1026+
;; ---------------------------------------------------------------------------
1027+
;; Tests for issue #6: cljs-eval-wire must distinguish "form returned nil"
1028+
;; from "shadow returned a blank response with no error" — the latter used
1029+
;; to surface as the misleading {:ok? true :value nil}.
1030+
;; ---------------------------------------------------------------------------
1031+
1032+
(deftest cljs-eval-wire-throws-on-blank-no-error-response
1033+
(testing "blank :value with no :err and no :ex throws :cljs-eval-empty (eval-op surfaces as {:ok? false ...})"
1034+
(with-redefs [ops/cljs-eval (fn [& _] {:value "" :err "" :ex nil :status #{:done}})]
1035+
(let [thrown (try (#'ops/cljs-eval-wire nil :app "(+ 1 2)" {})
1036+
(catch clojure.lang.ExceptionInfo e
1037+
{:msg (.getMessage e) :data (ex-data e)}))]
1038+
(is (= :cljs-eval-empty (-> thrown :data :reason))
1039+
":cljs-eval-empty distinguishes empty from legitimate-nil")
1040+
(is (contains? (:data thrown) :raw-response)
1041+
":raw-response carried for debugging")
1042+
(is (str/includes? (-> thrown :data :hint) "discover-app"))))))
1043+
1044+
(deftest cljs-eval-wire-passes-through-legitimate-nil-value
1045+
(testing "shadow returning :value \"nil\" (the EDN string) is a real nil result, not an empty response"
1046+
(with-redefs [ops/cljs-eval (fn [& _] {:value (pr-str {:results [(pr-str nil)]})
1047+
:status #{:done}})]
1048+
(let [result (#'ops/cljs-eval-wire nil :app "(prn :hi)" {})]
1049+
(is (nil? (:value result)) "legitimate nil passes through as :value nil")
1050+
(is (true? (:rfp.wire/value-fits? result)) "no error surfaced")))))
1051+
1052+
(deftest cljs-eval-wire-blank-with-err-keeps-existing-eval-error-path
1053+
(testing "blank :value WITH :err set is an eval error, not :cljs-eval-empty (parse-cljs-eval-response throws first)"
1054+
(with-redefs [ops/cljs-eval (fn [& _] {:value "" :err "" :ex "compile error somewhere"})]
1055+
(let [thrown (try (#'ops/cljs-eval-wire nil :app "(broken)" {})
1056+
(catch clojure.lang.ExceptionInfo e
1057+
(ex-data e)))]
1058+
(is (= :eval-error (:reason thrown)) "throws :eval-error from parse-cljs-eval-response, not :cljs-eval-empty")))))
1059+
1060+
(deftest cljs-eval-wire-blank-with-only-err-string-still-throws-cljs-eval-empty
1061+
(testing "blank :value with empty :err string but no :ex falls into :cljs-eval-empty (the headline #6 shape)"
1062+
(with-redefs [ops/cljs-eval (fn [& _] {:value "" :err nil :ex nil :status #{:done}})]
1063+
(let [thrown (try (#'ops/cljs-eval-wire nil :app "(+ 1 2)" {})
1064+
(catch clojure.lang.ExceptionInfo e
1065+
(ex-data e)))]
1066+
(is (= :cljs-eval-empty (:reason thrown)))))))
1067+
10261068
;; ---------------------------------------------------------------------------
10271069
;; Tests for issue #7: inject failures must surface as :inject-failed, not
10281070
;; pass through silently as nil from downstream ops.

0 commit comments

Comments
 (0)