mirror of
https://github.com/logseq/logseq.git
synced 2026-05-29 23:19:38 +00:00
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
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -66,19 +66,26 @@
|
||||
|
||||
(defn request
|
||||
[{:keys [method url headers body timeout-ms]}]
|
||||
(p/let [response (<raw-request {:method method
|
||||
:url url
|
||||
:headers headers
|
||||
:body body
|
||||
:timeout-ms timeout-ms})]
|
||||
(if (<= 200 (:status response) 299)
|
||||
(p/let [{:keys [body status] :as response}
|
||||
(<raw-request {:method method
|
||||
:url url
|
||||
:headers headers
|
||||
:body body
|
||||
:timeout-ms timeout-ms})]
|
||||
(if (<= 200 status 299)
|
||||
response
|
||||
(throw (ex-info (if (seq (:body response))
|
||||
(str "http request failed (" (:status response) ")\nhttp response: " (:body response))
|
||||
(str "http request failed (" (:status response) ")"))
|
||||
{:code :http-error
|
||||
:status (:status response)
|
||||
:body (:body response)})))))
|
||||
(let [parsed (when (and (= status 400) (seq body))
|
||||
(try (js->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]}
|
||||
|
||||
@@ -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))))))))
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user