-
-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
graphql2/error-stamper misses some global errors #152
Comments
Great debugging on this, your reasoning makes sense, if you send a PR with the fix I'll be glad to merge it. So seems we need to fix the error-stamper and the |
Hi, thank you for the kind words! If I fix this then I will break the current tests, here:
the test assumes that a query for
This example in the spec shows that the current code would be correct if we did not use aliases. So perhaps the question is: do we always create aliases when idents / parametrized fields are involved? I guess the answer is here: pathom/src/com/wsscode/pathom/graphql.cljc Line 108 in 7f5ebe0
Examples of the expected behaviorTwo examples from my practice: 1. An alias(parser {} [{[:channelapi.subscription/phoneNumber "98765432"]
[:channelapi.SubscriptionData/roles]}])
;;=>
;{[:channelapi.subscription/phoneNumber "98765432"] {},
; ::p/errors {[[:channelapi.subscription/phoneNumber "98765432"]]
; {:path ["_subscription_phoneNumber_98765432"],
; :message "INVALID_OR_EXPIRED_AUTH_TOKEN", ...}}} 2. Without an alias(parser {} [{:channelapi/subscriptions
[{[:channelapi.invoice/id "fakeid"] [:channelapi.Invoice/invoiceNumber]}]}])
;;=>
;{::p/errors {[:channelapi/subscriptions]
; {:path ["subscriptions"],
; :message "INVALID_OR_EXPIRED_AUTH_TOKEN", ...}}} |
When I run this query:
Pathom gets this response:
but the result the user gets is
i.e. the error is swallowed instead of being made into a reader error.
It should be caught and propagated by
com.wsscode.pathom.connect.graphql2/error-stamper
but it is not due to this part:When called, the input
errors
is{["_subscription_phoneNumber_98765432"] ...},
pathis [[:subscription/phoneNumber 98765432]]
, butpath'
becomes only[subscription]
instead of["_subscription_phoneNumber_98765432"]
. The error is thus not considered to be a "local error" and is ignored.A fix would be I guess to replace
(namespace (first %)))
with(com.wsscode.pathom.graphql/ident->alias %)
but I assume that the current code is there for a reason and is the right way to do it in other circumstances.I'd be more than happy to send a PR if you can guide me to as when to use
(namespace (first %)))
vs.ident->alias
.Follow-up error
When I fix
error-stamper
to detect the error, it will cause another error inpull-idents
(called fromgraphql-resolve
to process the result of(parser-item ...)
because it will be called withdata
like this:{[:subscription/phoneNumber "98765432"] :com.wsscode.pathom.core/reader-error}]
and it will fail withSo something needs to be changed there as well.
The text was updated successfully, but these errors were encountered: