From 105282cb78d7edc2d9022346bcc7c2444aae7daf Mon Sep 17 00:00:00 2001 From: Gabriel Horner Date: Tue, 24 Mar 2026 15:00:21 -0400 Subject: [PATCH] fix(cli): silently ignores several validation failures that only the desktop app sees. Ignoring validation errors than causes further errors as the cli doesn't see updates and doesn't understand the validation constraints it faces. For example, updating a block with a private tag e.g. `logseq upsert block --id=219 --update-tags='["Journal"]'` should fail with an explicit message instead of pretending to succeed --- src/main/frontend/worker/db_core.cljs | 4 +- src/main/frontend/worker/db_worker_node.cljs | 15 +++++++- src/main/logseq/cli/main.cljs | 6 +-- src/main/logseq/cli/transport.cljs | 31 ++++++++++------ .../frontend/worker/db_worker_node_test.cljs | 37 +++++++++++++++++++ src/test/logseq/cli/transport_test.cljs | 24 ++++++++++++ 6 files changed, 99 insertions(+), 18 deletions(-) diff --git a/src/main/frontend/worker/db_core.cljs b/src/main/frontend/worker/db_core.cljs index b9782c60f3..7e16e3ef35 100644 --- a/src/main/frontend/worker/db_core.cljs +++ b/src/main/frontend/worker/db_core.cljs @@ -1129,7 +1129,9 @@ :notification (do (log/error ::apply-outliner-ops-failed e) - (shared-service/broadcast-to-clients! :notification [(:message payload) (:type payload) (:clear? payload) (:uid payload) (:timeout payload)])) + (shared-service/broadcast-to-clients! :notification [(:message payload) (:type payload) (:clear? payload) (:uid payload) (:timeout payload)]) + ;; re-throw as CLI needs to see notification + (throw e)) (throw e))))))) (def-thread-api :thread-api/sync-app-state diff --git a/src/main/frontend/worker/db_worker_node.cljs b/src/main/frontend/worker/db_worker_node.cljs index 1607e4a485..6a4bebf7e7 100644 --- a/src/main/frontend/worker/db_worker_node.cljs +++ b/src/main/frontend/worker/db_worker_node.cljs @@ -120,7 +120,8 @@ {:method (or method-kw method-str) :elapsed-ms (- (js/Date.now) started-at)})) 10000)] - (-> (.remoteInvoke proxy method-str (boolean direct-pass?) args') + ;; wraps .remoteInvoke so synchronous throws become proper promise rejections with ex-data preserved + (-> (p/do! (.remoteInvoke proxy method-str (boolean direct-pass?) args')) (p/finally (fn [] (js/clearTimeout timeout-id)))))) @@ -256,10 +257,20 @@ {:ok true :resultTransit result}))))) (p/catch (fn [e] (let [data (ex-data e)] - (if (= :repo-locked (:code data)) + (cond + (= :repo-locked (:code data)) (send-json! res 409 {:ok false :error {:code :repo-locked :message (or (.-message e) "graph is locked")}}) + + ;; CLI should see same errors that app is seeing + (= :notification (:type data)) + (send-json! res 400 {:ok false + :error {:code :validation-failed + :message (or (get-in data [:payload :message]) + (.-message e))}}) + + :else (do (log/error :db-worker-node-http-invoke-failed e) (send-json! res 500 {:ok false diff --git a/src/main/logseq/cli/main.cljs b/src/main/logseq/cli/main.cljs index 3395064740..64a70a3926 100644 --- a/src/main/logseq/cli/main.cljs +++ b/src/main/logseq/cli/main.cljs @@ -70,14 +70,14 @@ (p/catch (fn [error] (let [data (ex-data error) message (cond - (and (= :http-error (:code data)) (seq (:body data))) - (str "http request failed (" (:status data) "): " (:body data)) + (some? (.-message error)) + (.-message error) (some? (:message data)) (:message data) :else - (or (.-message error) (str error)))] + (str error))] (if (= :data-dir-permission (:code data)) {:exit-code 1 :output (format/format-result {:status :error diff --git a/src/main/logseq/cli/transport.cljs b/src/main/logseq/cli/transport.cljs index 3d8f264cb7..39a0186aff 100644 --- a/src/main/logseq/cli/transport.cljs +++ b/src/main/logseq/cli/transport.cljs @@ -66,19 +66,26 @@ (defn request [{:keys [method url headers body timeout-ms]}] - (p/let [response (clj (js/JSON.parse body) :keywordize-keys true) + (catch :default _ nil))) + api-message (get-in parsed [:error :message]) + message (cond + (seq api-message) api-message + (seq body) (str "http request failed (" status ")\nhttp response: " body) + :else (str "http request failed (" status ")"))] + (throw (ex-info message + {:code :http-error + :status status + :body body})))))) (defn invoke [{:keys [base-url timeout-ms]} diff --git a/src/test/frontend/worker/db_worker_node_test.cljs b/src/test/frontend/worker/db_worker_node_test.cljs index 3743f1b1d8..a5dca07c1b 100644 --- a/src/test/frontend/worker/db_worker_node_test.cljs +++ b/src/test/frontend/worker/db_worker_node_test.cljs @@ -973,3 +973,40 @@ (-> (stop!) (p/finally (fn [] (done)))) (done)))))))) + +(deftest db-worker-node-validation-error-returns-400 + (async done + (let [daemon (atom nil) + data-dir (node-helper/create-tmp-dir "db-worker-validation-error") + repo (str "logseq_db_validation_" (subs (str (random-uuid)) 0 8))] + (-> (p/let [{:keys [host port stop!]} + (start-daemon! {:data-dir data-dir :repo repo}) + _ (reset! daemon {:stop! stop!}) + ;; Find a block to use as target and Journal tag's db/id + journal (invoke host port "thread-api/pull" + [repo [:db/id] [:db/ident :logseq.class/Journal]]) + journal-id (:db/id journal) + blocks (invoke host port "thread-api/q" + [repo + ['[:find ?e + :where + [?e :block/title] + [(missing? $ ?e :logseq.property/created-from-property)] + [(missing? $ ?e :block/name)]]]]) + block-id (ffirst blocks) + ;; Try to set the built-in Journal tag on the block + {:keys [status body]} + (invoke-raw host port "thread-api/apply-outliner-ops" + [repo [[:batch-set-property [[block-id] :block/tags journal-id {}]]] {}]) + parsed (js->clj (js/JSON.parse body) :keywordize-keys true)] + (is (= 400 status) + "validation errors should return 400, not 500") + (is (false? (:ok parsed))) + (is (string/includes? (get-in parsed [:error :message]) "Can't set tag") + "error message should describe the validation failure")) + (p/catch (fn [e] + (is false (str "unexpected error: " e)))) + (p/finally (fn [] + (if-let [stop! (:stop! @daemon)] + (-> (stop!) (p/finally (fn [] (done)))) + (done)))))))) diff --git a/src/test/logseq/cli/transport_test.cljs b/src/test/logseq/cli/transport_test.cljs index 4da7f52a1b..59068a1b96 100644 --- a/src/test/logseq/cli/transport_test.cljs +++ b/src/test/logseq/cli/transport_test.cljs @@ -108,6 +108,30 @@ (is false (str "unexpected error: " e)) (done)))))) +(deftest test-request-400-extracts-api-message + (async done + (-> (p/let [{:keys [url stop!]} (start-server + (fn [_req ^js res] + (.writeHead res 400 #js {"Content-Type" "application/json"}) + (.end res (js/JSON.stringify + #js {:ok false + :error #js {:code "validation-failed" + :message "Can't set tag with built-in #Journal"}}))))] + (p/catch + (transport/request {:method "POST" + :url (str url "/invoke") + :timeout-ms 1000}) + (fn [e] + (is (= :http-error (-> (ex-data e) :code))) + (is (= 400 (-> (ex-data e) :status))) + (is (= "Can't set tag with built-in #Journal" (ex-message e)) + "error message is the clean API message, not raw JSON"))) + (p/let [_ (stop!)] true)) + (p/then (fn [_] (done))) + (p/catch (fn [e] + (is false (str "unexpected error: " e)) + (done)))))) + (deftest test-request-timeout (async done (-> (p/let [{:keys [url stop!]} (start-server