Skip to content

Commit

Permalink
Expose missing attribute details
Browse files Browse the repository at this point in the history
  • Loading branch information
@wilkerlucio
wilkerlucio committed Dec 5, 2024
1 parent cd0fe5d commit 0942777
Show file tree
Hide file tree
Showing 6 changed files with 383 additions and 149 deletions.
9 changes: 0 additions & 9 deletions src/main/com/wsscode/pathom3/connect/planner.cljc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -575,15 +575,6 @@
op-name
(update-in [::index-resolver->nodes op-name] coll/sconj node-id))))

(defn create-and [graph env node-ids]
(if (= 1 (count node-ids))
(get-node graph (first node-ids))
(let [{and-node-id ::node-id
:as and-node} (new-node env {})]
(-> graph
(include-node and-node)
(add-node-branches and-node-id ::run-and node-ids)))))

(defn create-root-and [graph env node-ids]
(if (= 1 (count node-ids))
(set-root-node graph (first node-ids))
Expand Down
94 changes: 80 additions & 14 deletions src/main/com/wsscode/pathom3/connect/runner.cljc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
[com.wsscode.pathom3.placeholder :as pph]
[com.wsscode.pathom3.plugin :as p.plugin]))

; region specs

(>def ::attribute-errors (s/map-of ::p.attr/attribute any?))
(>def ::attributes-missing ::pfsd/shape-descriptor)

Expand Down Expand Up @@ -130,6 +132,8 @@
for resolver nodes."
boolean?)

; endregion

(>defn all-requires-ready?
"Check if all requirements from the node are present in the current entity."
[env {::pcp/keys [expects]}]
Expand Down Expand Up @@ -467,7 +471,38 @@
::pcp/foreign-ast (::pcp/foreign-ast node)}
input-data))

(defn wait-batch-check
(defn input-missing-error-message [nodes attr]
(if (> (count nodes) 1)
(str "- Attribute " attr " was expected to be returned from resolvers "
(str/join ", " (distinct (map ::pco/op-name nodes)))
" but all of them failed to provide it.")
(str "- Attribute " attr " was expected to be returned from resolver " (::pco/op-name (first nodes)) " but it failed to provide it.")))

(defn input-missing-detail-OR
[{::pcp/keys [graph]} or-node attr]
(let [resolver-nodes (->> (pcp/node-successors graph (::pcp/node-id or-node))
(map #(pcp/get-node graph %))
(filter #(and
(contains? (::pcp/expects %) attr)
(::pco/op-name %))))]
(input-missing-error-message resolver-nodes attr)))

(defn input-missing-detail
[{::pcp/keys [graph] :as env} node attr]
(if-let [missed-parent (->> (pcp/node-ancestors graph (::pcp/node-id node))
(map #(pcp/get-node graph %))
(filter #(contains? (::pcp/expects %) attr))
first)]
(if (::pcp/run-or missed-parent)
(input-missing-detail-OR env missed-parent attr)
(input-missing-error-message [missed-parent] attr))
(str "Can't find parent node that should provide attribute " attr ", this is likely a bug in Pathom, please report it.")))

(defn input-missing-check
"This will verify if all dependencies required by a resolver are satisfied.
One special case here might be a dependency from a batch process that's pending to run. In this case,
we return a special block of data that tells Pathom to wait for the batch to run and try it again."
[env node entity input input+opts]
(let [missing-all (pfsd/missing-from-data entity input+opts)
wait-batch? (and missing-all
Expand All @@ -480,7 +515,9 @@
(wait-batch-response env node)

missing
::node-error)))
(do
(merge-node-stats! env node {::node-missing-required-inputs missing})
::node-error))))

(defn invoke-resolver-from-node
"Evaluates a resolver using node information.
Expand Down Expand Up @@ -508,10 +545,10 @@
_ (merge-node-stats! env node
{::resolver-run-start-ms (time/now-ms)})
response (try
(let [batch-check (wait-batch-check env node entity input input+opts)]
(let [missing-check (input-missing-check env node entity input input+opts)]
(cond
batch-check
batch-check
missing-check
missing-check

batch?
(if-let [x (p.cache/cache-find resolver-cache* [op-name input-data params])]
Expand Down Expand Up @@ -645,14 +682,14 @@
[{::keys [node-run-stats*]} {::pcp/keys [node-id]} taken-path-id]
(refs/gswap! node-run-stats* update-in [node-id ::taken-paths] coll/vconj taken-path-id))

(defn fail-or-error [or-node errors]
(defn or-error-exception [or-node errors]
(ex-info
(str "All paths from an OR node failed. Expected: " (::pcp/expects or-node) "\n"
(str/join "\n" (mapv ex-message errors)))
{:errors errors}))

(defn handle-or-error [env or-node res]
(let [error (fail-or-error or-node (::or-option-error res))]
(let [error (or-error-exception or-node (::or-option-error res))]
(mark-node-error-with-plugins env or-node error)
(fail-fast env error)))

Expand All @@ -677,16 +714,17 @@
(let [picked-node-id (choose-path env or-node nodes)
node-id (if (contains? nodes picked-node-id)
picked-node-id
(do
(let [actual-node (first nodes)]
(l/warn ::event-invalid-chosen-path
{:expected-one-of nodes
:chosen-attempt picked-node-id
:actual-used (first nodes)})
(first nodes)))]
:actual-used actual-node})
actual-node))]
(add-taken-path! env or-node node-id)
(let [res (try
(run-node! env (pcp/get-node graph node-id))
(catch #?(:clj Throwable :cljs :default) e

{::or-option-error e}))]
(cond
(::batch-hold res)
Expand All @@ -704,7 +742,7 @@
(::batch-hold res)
res

(and (::or-option-error res)
(and (seq (::or-option-error res))
(not (or-expected-optional? env or-node)))
(handle-or-error env or-node res)

Expand Down Expand Up @@ -830,9 +868,37 @@
(doseq [ast (::pcp/mutations graph)]
(invoke-mutation! env ast)))

(defn entity-missing-attribute-details
[{::pcp/keys [graph]
::keys [node-run-stats*]
:as env}
attr]
(let [{nodes-missing-inputs true
nodes-missing-attribute-in-response false}
(->> (get-in graph [::pcp/index-attrs attr])
(map #(pcp/get-node graph %))
(group-by #(or (contains? (get @node-run-stats* (::pcp/node-id %)) ::node-missing-required-inputs)
(not (contains? @node-run-stats* (::pcp/node-id %))))))]
(str/join
"\n"
(cond-> []
(seq nodes-missing-attribute-in-response)
(conj (input-missing-error-message nodes-missing-attribute-in-response attr))

(seq nodes-missing-inputs)
(into
(map (fn [{::pcp/keys [node-id input]
::pco/keys [op-name]
:as node}]
(let [missing (or (get-in @node-run-stats* [node-id ::node-missing-required-inputs])
(pfsd/missing-from-data (p.ent/entity env) input))]
(str "- Attribute " attr " was expected to be returned from resolver " op-name " but dependencies were missing:\n"
(str/join "\n" (map #(str " " (input-missing-detail env node %)) (keys missing)))))))
nodes-missing-inputs)))))

(defn check-entity-requires!
"Verify if entity contains all required keys from graph index-ast. This is
shallow check (don't visit nested entities)."
a shallow check (don't visit nested entities)."
[{::pcp/keys [graph]
:as env}]
(let [entity (p.ent/entity env)
Expand All @@ -847,8 +913,8 @@
(fail-fast env
(ex-info (str "Required attributes missing"
(p.path/at-path-string env)
": "
(pr-str (vec (keys missing))))
":\n"
(str/join "\n" (map #(entity-missing-attribute-details env %) (keys missing))))
{::attributes-missing missing
::p.error/phase ::execute
::p.error/cause ::p.error/attribute-missing})))))
Expand Down
10 changes: 6 additions & 4 deletions src/main/com/wsscode/pathom3/connect/runner/async.cljc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,12 @@
resolver-cache* (get env cache-store)
_ (pcr/merge-node-stats! env node
{::pcr/resolver-run-start-ms (time/now-ms)})
batch-check (pcr/wait-batch-check env node entity input input+opts)
missing-check (try
(pcr/input-missing-check env node entity input input+opts)
(catch #?(:clj Throwable :cljs :default) e (p/rejected e)))
response (-> (cond
batch-check
batch-check
missing-check
missing-check

batch?
(if-let [x (p.cache/cache-find resolver-cache* [op-name input-data params])]
Expand Down Expand Up @@ -327,7 +329,7 @@
(::pcr/batch-hold res)
res

(and (::pcr/or-option-error res)
(and (seq (::pcr/or-option-error res))
(not (pcr/or-expected-optional? env or-node)))
(pcr/handle-or-error env or-node res)

Expand Down
8 changes: 5 additions & 3 deletions src/main/com/wsscode/pathom3/connect/runner/parallel.cljc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,10 @@
resolver-cache* (get env cache-store)]
(pcr/merge-node-stats! env node
{::pcr/resolver-run-start-ms (time/now-ms)})
(p/let [response (-> (if (pfsd/missing-from-data entity input)
::pcr/node-error
(p/let [response (-> (if-let [missing (pfsd/missing-from-data entity input)]
(do
(pcr/merge-node-stats! env node {::pcr/node-missing-required-inputs missing})
::pcr/node-error)
(cond
batch?
(if-let [x (p.cache/cache-find resolver-cache* [op-name input-data params])]
Expand Down Expand Up @@ -409,7 +411,7 @@
(::pcr/batch-hold res)
res

(and (::pcr/or-option-error res)
(and (seq (::pcr/or-option-error res))
(not (pcr/or-expected-optional? env or-node)))
(pcr/handle-or-error env or-node res)

Expand Down
23 changes: 3 additions & 20 deletions src/main/com/wsscode/pathom3/error.cljc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,24 +118,7 @@
(defn error-stack [err]
(gobj/get err "stack")))

(defn datafy-processor-error* [env]
(-> env
(select-keys [::error-data
::error-message
::error-stack
::pcp/graph
:com.wsscode.pathom3.connect.runner/processor-error?
;:com.wsscode.pathom3.connect.runner/processor-error-parent-env
:com.wsscode.pathom3.entity-tree/entity-tree
:com.wsscode.pathom3.path/path])
(coll/update-if
:com.wsscode.pathom3.connect.runner/processor-error-parent-env
datafy-processor-error*)))

(defn datafy-processor-error [^Throwable err]
(let [env (ex-data err)]
(if (some-> env :com.wsscode.pathom3.connect.runner/processor-error?)
(datafy-processor-error* env)
{::error-message (ex-message err)
::error-data (ex-data err)
::error-stack (error-stack err)})))
{::error-message (ex-message err)
::error-data (ex-data err)
::error-stack (error-stack err)})
Loading

0 comments on commit 0942777

Please sign in to comment.