Strict mode

[wilkerlucio]

Overview: Pathom moved to strict mode by default. If you want to keep the old behavior set the attribute :com.wsscode.pathom3.error/lenient-mode? true in your env. Docs with more details will be available soon.

Previous content:

Pathom 3 now has a configuration to fail immediately upon exceptions.

Setting the key ::pcr/fail-fast? true will make exceptions on resolvers and mutations to flow all the way up, instead of being captured as per default behavior.

This an experimental feature and I like to gather feedback from the community about it.

Considering that fail fast is more intuitive for new users, and also generally a better response in dev, should it be enabled by default?

Any other comments or suggestions, please enter the discussion here.

[lgessler]

Casual user here: agree that fail fast would have been more intuitive for me when I was learning Pathom. Enabling this by default would have made me a happier new user unless there's risk of really bad things happening in prod if it's not turned off.

[souenzzo]

In pathom2 I use failfast just on mutations.

If I have a mutation with a join read, i dont want to have a fail from the read.

[awkay]

I think this is a very important point.

[wilkerlucio]

That makes sense, and there isn't any mode in which Pathom runs mutation join over a failed mutation, in any mode. If you find this behavior please report a bug.

[wilkerlucio]

I've been thinking about this, and I'm now thinking that would be great to have a more "strict" version pathom, and that should be the default.

When I built the previous versions, my use cases tend to need more tolerant modes, and I still want to support them.

Now after hearing more community cases and doing some of those myself, I believe a version that "fails soon" will be beneficial.

This is what I'm thinking to change from ::pcr/fail-fast? to :com.wsscode.pathom3.system/loose-mode?, which will be false by default. With loose-mode on, Pathom 3 will behave as it does now, minimizing the chance of a crash and trying to provide anything it can while keeping data about failures on the way.

With tolerant mode as false, these are the changes:

• If an attribute fails to plan, Pathom already throws an exception and stop before it even calls any resolver
• If a resolver doesn't return all expected attributes from it, throw it immediately
• Support optional attributes also in the EQL process, this way we can make some parts of the query tolerant to failure by keeping the default as strict
• OR nodes are special, they only fail if all options fail, and in that case throws a special exception that contains the errors from all the paths

Would love to hear what you all think about those changes.

[souenzzo]

I'm still confused if will be possible to have different policies for read/mutations?!

What it we have a predicate to choose which exceptions should be tolerated?

We can have a design like this:
https://clojure.org/reference/repl_and_main#_error_printing
with a (-> ex ex-data ::p/ex-phase), we can do a simple case and choose which phases matters.

[wilkerlucio]

the initial idea is to not have the difference, I wonder if we separate the semantic wont be a little confusing. in terms of code its easy to do. Maybe we can figure a way to make this possible via some plugin extension. so far I won't have something like that as default, but I would like to make this possible in some way.

[souenzzo]

for a simple default behavior, "fail-fast" seems to be better for me.

[wilkerlucio]

Working on things at branch #67 in case anyone wants to give it a try.

[markaddleman]

I think this is a good change because it fits my workflow style. As I'm developing resolvers, I'd like to be aware of bugs in resolvers or backends as soon as possible. As I grow confident in each resolver / backend, I will want to opt for a more error tolerant (ie, "loose") system.

[wilkerlucio]

Status update, most of it seems to work fine. I'm excited, its really fun to use this mode, with all the errors surfacing early.

One new thing is that now you can use optional inputs at the p.eql/process to indicate to Pathom that you are ok with that attribute not working, for example:

(p.eql/process (pci/register
                 [(pco/resolver 'foo
                    {::pco/output [:foo]}
                    (fn [_ _] {:foo "bar"}))])
  {}
  [:foo :not-available])
; throws => Pathom can't find a path for the following elements in the query: [:not-available] at path []

; now this one doesn't throw:
(p.eql/process (pci/register
                 [(pco/resolver 'foo
                    {::pco/output [:foo]}
                    (fn [_ _] {:foo "bar"}))])
  {}
  [:foo (pco/? :not-available)])

There is one problem I'm trying to get around though. The issue is when a resolver is a leaf resolver (no nodes after) and it doesn't return the expected key (but also didn't throw any errors), for example:

(p.eql/process (pci/register
                 (pco/resolver 'foo
                   {::pco/output [:foo]}
                   (fn [_ _] {})))
  {}
  [:foo])

With the current code, this returns an empty map. But I like to make the strict to be really about "required", so I think the better behavior is for that to throw. My current solution is to run a check after the entity is ready to verify if all required attributes are present in the output. This works in almost every case, except for nested optional inputs. In this scenario, Pathom throws an error when that optional attribute is not there. Getting this to work is being a bit harder than I anticipated, but it's the current thing in progress.

[eneroth]

Consider modelling a tree, where each node either has :children or :value, or perhaps a resolver user-id->settings, where not every setting is applicable to every user. This would force an enumeration of every possible value, and force us to nil those values which are not present.

I'm wondering if this doesn't start edging into the domain of specs, perhaps. Not that it wouldn't be useful to be able to write a spec for a resolver. Just that treating output as strictly equivalent to required might be semantically a bit tenuous in some circumstances. It might be a bit as if Spec had :req but no :opt.

[wilkerlucio]

In my head, the thing here is about where the optionality should live, in the provider or in the consumer? Currently, we have optionality in the consumer (inputs OR requests). Adding optional on the output means expressing this at the provider level. I wonder if should be a thing, or in other terms: what abilities we gain with this information?

One use case I can think of is when prioritizing resolvers, maybe we prefer a resolver that says it can always return a value vs one that may return the value.

But even with that, Pathom still has to deal with the consumer optionality, for instance, if :foo depends on :bar, and there is a resolver for :bar. Doesn't matter if :bar is optional or not at the output definition, Pathom still must call it to try to get. And also, being optional or not doesn't affect if an error should happen (either if there is :bar or not in the output), because what matters is if the caller requires it or not (if :bar is optional or not in terms of :foo).

These are my current thoughts on it, so far seems like an unknown area, not seeing many benefits so far, also with the risk of adding unexpected bugs. As it is I wouldn't do it, but if you have other arguments for it I'll love to hear.

[wilkerlucio]

The problem just got fixed! If you want to understand a bit more, I'll like to explain how nested input resolution works, and how this fix also improves that part of Pathom 3 runner.

To explain this, let's consider the following setup:

(pco/defresolver top-ranked []
  {:players/top-ranked
   [{:player/name  "A"
     :player/score 10}
    {:player/name  "B"
     :player/score 19}
    {:player/name  "C"
     :player/score 14}
    {:player/name  "D"
     :player/score 18}]})

(pco/defresolver highest-score
  [{:players/keys [top-ranked]}]
  {::pco/input
   [{:players/top-ranked
     [:player/score]}]}
  {:players/highest-score
   (apply max (mapv :player/score top-ranked))})

(p.eql/process
  (pci/register
    [top-ranked
     highest-score])
  [{:players/top-ranked [:player/name]}
   :players/highest-score])
; => {:players/top-ranked [{:player/name "A"}
                      {:player/name "B"}
                      {:player/name "C"}
                      {:player/name "D"}],
 :players/highest-score 19}

The way Pathom deals with this is at the planner level, when computing the requirements for :players/highest-score, Pathom will extend the user query to also include what it needs from :players/top-ranked.

; at planner level, Pathom turns the user query from
[{:players/top-ranked 
  [:player/name]}
 :players/highest-score]

; to
[{:players/top-ranked 
  [:player/name
   :player/score]}
 :players/highest-score]

; so the request fulfill the :players/highest-score needs to load the :player/score

The check I add to verify if all the required fields are fulfilled takes into consideration if the attribute was marked as optional (using pco/?). This made it work fine for flat queries.

The issue with the nested queries is that the AST extension wasn't taking into consideration if the nested input was required or optional. By moving this information up and extending the AST including this, the tests now pass and it works as expected.

Excited to keep playing with this, I hope to merge it soon!

[eneroth]

Thanks for the explanation! I guess I'm mixing up required inputs and required outputs in my head 😅

I was looking at our own code and considering that, for example, we have a resolver that can return either a value or a relation. When it's a relation, I need to stick use another query than if it's a plain value, in order to traverse the relation, and I don't know beforehand if a particular entity will return one or the other (but there can never be both on the same entity).

[wilkerlucio]

Not sure if I get the issue here, can you give an example with a few resolvers to demo it?

[wilkerlucio]

Also happy to hear feedback about it, you can use it by pointing to the branch directly. Good time to check and see if you find any issues with the approach.

[wilkerlucio]

Another important change to mention, I'll remove the attribute-errors-plugin, when using lenient mode it will do the same thing automatically (no need to setup a plugin).

[wilkerlucio]

Now I'm considering one issue with this idea. We may also need this kind of error tracking for optional attributes. So maybe attribute error tracking should be enabled at all times?

[markaddleman]

I think there's a difference between these two cases:

1. an optional attribute that cannot be computed because its resolver returns nil or whose inputs cannot be satisfied
2. an optional attribute whose resolver throws an exception

Intuitively, I'd want a "strict mode" to fail for case #2 in the same way that it would for a non-optional attribute.

Perhaps there should be an additional mode to specify the level of strictness around optional attributes?

[wilkerlucio]

Thanks for the input. I agree, in the case of 2, it will throw. Additional modes we may consider in the future, for now, let's see how this method goes.

[awkay]

So here are some thoughts:

Inside of the logic of a resolver that can be many reasons I don’t want to return a requested property: Current access rights might cause me to elide certain information, the data might be temporarily unavailable (think of async cache updates, like Elasticsearch), etc. I don’t want the resolver’s choice or inability to resolve data to generate errors. That just doesn’t make sense in those use-cases. Elision is an acceptable choice of implementation, and transient conditions in my infrastructure should also not generate errors. Most people do not thing about things like transient infrastructure during development, and this leads to subtle bugs and faulty behavior. That said, having an app completely fail on a request that has a subtle bug is probably fine. Just pointing out trade-offs.

Having a client define what might be optional is very counter-intuitive. The client can’t actually dictate what the server is capable of, and so you’re asking for a mismatch to be accidentally written, or evolve over time. I declare it isn’t optional in the UI at some point when that is true. I change the server. It is very easy for the tests and dev to still pass (they happen to supply data), and then fail later in production. This kind of thing happens all the time. The core problem is that the synchronization of the truth of the optionality is duplicated. It’s codified on the server, and the copied to the client. This is why RAD centralizes these kinds of things in the actual definition of attributes (e.g. ao/required? true ) in CLJC so the front/back end have to agree on it by definition. Generating resolvers and UI from this common ground means that the two cannot possibly mismatch. This is a much saner approach IMO, but of course that is more of what RAD does as opposed to Pathom.

In terms of the actual usefulness of what you’re talking about, be careful about demarcating things that are primarily useful in development mode vs production. I can see the temptation in adding these things as development convenience for catching errors. That said, think about these cases you care about for that:

1. You make a typo in client. Pathom can easily detect no output is possible and throw. This is always useful and requires no new syntax, schema, etc. Schema (which is what optionality is part of) can be a real slippery slope. Imagine an invoice where something is optional based on payment type (credit needs credit card). This kind of data dependency isn't addressed, but is real.
2. You put something in an unreachable part of the path. Again, Pathom can do analysis and tell you the attribute isn't reachable. Period.
3. The resolver returns a mis-spelled prop. Auto-detection is possible again.

Declaring something non-optional and having that throw an error when a resolver doesn't return it has a lot of possible downsides, and I think if you run through the above 3 items you get most (all?) of the benefit without needing this extra syntax.

Disclaimer...I entered this convo via DM with Wilker on Slack, so I didn't read the entire thread because I think I had the full context. This may not be true :)

[wilkerlucio]

And since I have you here with all this context in mind. I also made it possible for the client to turn on/off the lenient mode. In the new Boundary interface the client can send :pathon/lenient-mode? true/false to force a mode. I'm not super confident this is a good idea yet, it's out there, but subject to change. How does that option feel to you? Should the client be allowed to make this decision? Or the server should impose? (Its also relative easy for a server to disable this, by wrapping the boundary interface and dissoc that key, but users need to be aware this is a thing)

[awkay]

"The client can send..." How? Your docs don't mention it. As a parameter? At what level?

And since the client can just say everything is optional at the EQL layer, then yes, it totally makes sense. It is a convenience, not a security hole or something. They can already express it, this just makes it easier. Unless I'm missing something.

[wilkerlucio]

The boundary interface is like the new parser, in reality, it's a wrapper around p.eql/process. It supports both the standard pure EQL format or a map format, in which the client can specify more things, like the initial entity data. For example:

(def pathom (p.eql/boundary-interface env))

; request EQL
(pathom [::now])

; send root entity data
(pathom {:pathom/entity {:geo/width 10 :geo/height 8}
         :pathom/eql    [:geo/area]})

; use AST, this way Pathom doesn't have to decode the EQL
(pathom
  {:pathom/ast {:type     :root
                :children [{:type         :prop
                            :key          ::now
                            :dispatch-key ::now}]}})

So I made an extension on the boundary interface to forward the :pathom/lenient-mode? param to the env.

[wilkerlucio]

Pathom 2.4.0+ also supports this format in the parser. (Recent versions of Pathom Viz use that map format to send root entity data from the UI).

[awkay]

"to forward the param" is what is confusing. I think you mean "to forward the key/value pair from the map to the env"

[wilkerlucio]

Documentation changes related to strict mode wilkerlucio/pathom3-docs#25

[eneroth]

Would it be possible to work in strict mode most of the time, and mark some returned attributes as optional? I.e., similar to pco/? for inputs.

[eneroth]

It's pretty similar to what you wrote above. For example,

[:atom/particles 
 [:particle/value 
  {:particle/relation '...}]]

(In practice, we don't use recursive queries here, I'm just illustrating that the definition is recursive)

[wilkerlucio]

I'm trying to understand the variants you talked about, the specific attributes inside like :number-of-antennae

[eneroth]

The above is the simplest example I can think of: the frontend doesn't know whether value or relation will be present, but it can rely on one of them being present. For relation, it uses a different subquery than for value.

If you want a more advanced example, I previously worked with querying a set of JATS encoded scientific articles. Oh boy, the definition of a JATS document consists almost entirely of optional (and sometimes, mutually exclusive) attributes. It would be very hard to query such a collection through Pathom, if optional is frowned upon.

I'm not saying that it would necessarily be a goal for Pathom to be able to express queries over such a collection. :) (Nevertheless, I got it to work somehow, using Pathom 2 at the time)

[wilkerlucio]

If their thing is having optional as the norm, lenient mode seems more suitable. So far doesn't seem to make sense to bend strict too much trying to accommodate such cases. Let's set on that for now.

[eneroth]

That is certainly fair enough. In my use case, everything is strict in principle, with the exception of those two annoying attributes.

[souenzzo]

Hello

Reading strict-mode section, I missed a programmatic description of the error.

Inspired by clojure REPL error i tried to categorize each exception with a cause, phase, and an anomaly.

; Attribute doesn't exist in the indexes
(p.eql/process
  (pci/register
    (pbir/constantly-resolver :a "foo"))
  [:b])
; => EXCEPTION: Pathom can't find a path for the following elements in the query: [:b] at path []
#_{:cognitect.anomalies/category :cognitect.anomalies/unsupported
   ::p/phase                     ::p/plan
   ::p/cause                     ::p/find-attribute}

; Attribute exists in the indexes, but the available data isn't enough to reach it
(p.eql/process
  (pci/register
    (pbir/single-attr-resolver :a :b "foo"))
  [:b])
; => EXCEPTION: Pathom can't find a path for the following elements in the query: [:b] at path []
#_{:cognitect.anomalies/category :cognitect.anomalies/unavailable
   ::p/phase                     ::p/plan
   ::p/cause                     ::p/find-path}

; An extended version of this is also true in case of nested inputs, if Pathom can't see
; a Pathom for a nested input requirement, it will consider it a plan failure.
(p.eql/process
  (pci/register
    [(pco/resolver 'nested-provider
       {::pco/output [{:a [:b]}]}
       (fn [_ _]
         {:a {:b 1}}))
     (pco/resolver 'nested-requires
       {::pco/input  [{:a [:c]}]
        ::pco/output [:d]}
       (fn [_ _]
         {:d 10}))])
  [:d])
; => EXCEPTION: Pathom can't find a path for the following elements in the query: [:c] at path [:a]
#_{:cognitect.anomalies/category :cognitect.anomalies/unavailable
   ::p/phase                     ::p/plan
   ::p/cause                     ::p/find-path}

; Attribute wasn't in the final output after all the process
(p.eql/process
  (pci/register
    (pco/resolver 'data
      {::pco/output [:a :b :c]}
      (fn [_ _]
        {:a 10})))
  [:c])
; => EXCEPTION: Required attributes missing: [:c] at path []
#_{:cognitect.anomalies/category :cognitect.anomalies/unavailable
   ::p/phase                     ::p/execute
   ::p/cause                     ::p/find-path}

; A resolver throw an exception
(p.eql/process
  (pci/register
    (pco/resolver 'error
      {::pco/output [:error]}
      (fn [_ _]
        (throw (ex-info "Deu ruim." {})))))
  [:error])
; => EXCEPTION: Deu ruim.
#_"do not wrap, just let it throw"

It would help to develop a programmatic way to customize all these new behaviors

[markaddleman]

I'd like to make the case for always failing queries when a resolver throws an exception even in when included in OR nodes. In my usecase, I have a few resolvers that accept different inputs and produce the same output attribute. These resolvers are used to "package" data that the UI should treat as black-box and simply return to the server in subsequent queries. These resolvers are ordered using priorities. Pathom plans include these resolvers as siblings in OR nodes.

Under the current behavior, an exception in a high priority resolver causes the next resolver to execute. It's possible that the second resolver's inputs are a subset of the first so it can execute successfully but this is not desirable. It either results in a logic error (because an incomplete set of data is packaged up) or as a performance issue (because the second resolver is able to obtain the missing data through some I/O operation).

[wilkerlucio]

Hello Mark, throwing in an optional fail on an OR node would break Pathom semantics. The semantics I pick for the optionality (in terms of multiple options for a single attribute) has always been as "multiple ways for the same data". This semantic I won't change because then it would change all the expectations around that. Them it's a game of finding the right modeling taking this into consideration.

[markaddleman]

Fair enough. I'm very open to the idea that I've modeled this badly. I'll open a separate discussion

[markaddleman]

One additional comment on this: I completely agree on "multiple ways for the same data" semantics in a production context. In development, however, I think failing on a resolver exception would be convenient.

[wilkerlucio]

Fair enough. I'm very open to the idea that I've modeled this badly. I'll open a separate discussion

No worries on that, we are all still figuring this out :)

One additional comment on this: I completely agree on "multiple ways for the same data" semantics in a production context. In development, however, I think failing on a resolver exception would be convenient.

Not sure about that, because this means you are ignoring these paths at development time.

[wardle]

Hi all. This is an interesting discussion. Do you think that issues of optionality in the client are orthogonal to navigating a graph? I will give an example. In my graph, I have resolvers that run from a patient to an encounter to the hospital site at which that encounter was held to many more nodes off that. As you might imagine, that graph connects multiple disparate sources of information including a range of reference data. There will be encounters that are based in the community. The absence of hospital is a feature of the domain - it is optional. But I am now forcing client applications and the EQL to not only traverse a graph but impose their own schema by declaring that optionality up-front. So it is unclear to me why you new 'strict' mode. Surely you compose strictness by using a schema together with your graph navigation? I could use clojure.spec to vouch for the return data? Handling exceptions in attribute resolution is important; for some we can continue without that attribute - but for many circumstances, domain-specific, you might want to pretend nothing was resolved. Mark PS Obviously I can use lenient mode for my set-up, but pathom helps me handling real-life messy data and one of the attractions of clojure, and pathom, has been not worrying quite as much about optionality and types as I used to have to do in java - so I write code that handles nils more transparently.

[wilkerlucio]

Hello Mark,

As I discussed with Tony Kay, strict mode is about providing the same semantics Pathom already had in resolver inputs (which were always strict, with possibility for optionality) to the outside query. This is mostly useful for applications using Pathom in-process, which is part of my work to broaden Pathom usages outside of just API contexts.

That said, the lenient mode isn't a temporary feature, it's an important one for users with large queries and API usages.

But you still have questions or concerns, please let me know.

[wardle]

Thanks Wilker. The general abstraction that subsumes strict and lenient mode would be signalling a condition - acting upon that condition (or not) is, I think, domain and context specific, and may, or may not, be acted or inferred upon. For many of my uses, an unresolvable attribute is definitely not an error, but I'll still want to flag an error if the absence is due to a service problem not simply missing data, while sometimes, missing data is an error (ie falling into schema / clojure.spec territory).