Fork me on GitHub
#pathom
<
2020-03-20
>
wilkerlucio00:03:26

@kwladyka you can set ::p/process-error in the env, this allows you to process the error in any way you want to return to the user, example: (p/parser {::p/env {::p/process-error (fn [env error] error)}}) ; returns the literal error

wilkerlucio00:03:51

by default Pathom uses a string representation due to the initial usages always crossed the wire, so that ended up becaming the default

👍 4
wilkerlucio00:03:08

but as experience shows, its rare that this default is the best option

kwladyka13:03:13

(def parser
  (p/parallel-parser
    {::p/env {::p/reader [p/map-reader
                          pc/parallel-reader
                          pc/open-ident-reader
                          p/env-placeholder-reader]
              ::p/process-error (fn [env error] (println "foo" error))
              ::p/placeholder-prefixes #{">"}}
     ::p/mutate pc/mutate-async
     ::p/plugins [(pc/connect-plugin {::pc/register resolvers})
                  p/elide-special-outputs-plugin
                  p/error-handler-plugin
                  p/trace-plugin]}))
it doesn’t work for me. ::p/process-error never triggered

cjmurphy13:03:24

This plugin (put under ::p/plugins) works for me.

souenzzo13:03:28

p/error-handler-plugin should be before p/elide-special-outputs-plugin

👍 4
kwladyka13:03:41

@souenzzo still doesn’t work when I return map . ::p/process-error work when I return string

wilkerlucio13:03:13

check if it isn't some encoding issue, but if you run on the REPL it should give the correct answer

wilkerlucio17:03:13

strange, what is getting out on the error when you try to use a map?

kwladyka18:03:09

there is no error, but also no data, so {} is returned

kwladyka18:03:20

I have to add :logs to ::pc/output to see this data

kwladyka18:03:37

but if not, then there is no error and no data

kwladyka18:03:45

{:logs :com.wsscode.pathom.core/not-found, :shops :com.wsscode.pathom.core/not-found}

kwladyka19:03:36

@wilkerlucio any idea how can I return this :logs without adding to ::pc/output in each resolver?

kwladyka19:03:11

I don’t want to throw exception for things which I predicted. This is well precited error in code, so no reason to throw exception.

kwladyka19:03:53

this is the thing which probably we misunderstand (maybe)

kwladyka19:03:18

at least pathom works like that

kwladyka19:03:06

I want to return :logs even if not in ::pc/output and even if not listed in EQL query

wilkerlucio19:03:01

@kwladyka seems like you are talking about something different than error processing

wilkerlucio19:03:17

if you don't likst things on ::pc/output, the resolver is not going to get called

kwladyka19:03:40

sure and this is fine

wilkerlucio19:03:40

and I dont see any way around that, this is a very basic premise of pathom, because this is how things are indexed and how pathom decides what to trigger

kwladyka19:03:42

I will show you:

kwladyka19:03:57

this is ver. which I have now:

wilkerlucio19:03:58

but maybe I'm not understading you yet

kwladyka19:03:09

(pc/defresolver get-shops [env {:shop/keys [uuids]}]
  {::pc/input #{:shop/uuids}
   ::pc/output [{:shops [:shop/uuid :shop/name :shop/engine :shop/config]}]}
  (let [shops (shop-db/get-shops-by-uuid (case uuids
                                           :all :all
                                           (set (map (partial safe-coercion uuid/as-uuid) uuids))))]
    (if (vector? shops)
      {:shops (mapv #(update % :shop/uuid str) shops)}
      shops)))

kwladyka19:03:32

BUT if during processing all of this there is for example error in validation then this resolver return:

kwladyka19:03:42

{:logs [{:type :error,
         :code :validation,
         :cause ({:via [:shop/uuids :shop/uuid],
                  :val "999",
                  :message "UUID has to be in valid format (for example 00000000-0000-0000-0000-000000000000)."}
                 {:via [:shop/uuids],
                  :val #{"999"},
                  :message "Has to be set of uuid (for example #{00000000-0000-0000-0000-000000000000})."})}]}

kwladyka19:03:51

instead of :shops

kwladyka19:03:08

so the resolver doesn’t have to be called for :logs request

kwladyka19:03:21

I just want to have 1 unified clear way of returning errors

kwladyka19:03:32

but not exceptions, errors which my code predict and can react

wilkerlucio19:03:40

that's quite a different approach, nothing wrong with that, but requires you to re-invent the error system

kwladyka19:03:44

exceptions are things which goes wrong

wilkerlucio19:03:47

pathom errors are just data in the end

wilkerlucio19:03:09

if you want something like that, you have to make your own plugin

wilkerlucio19:03:20

add an atom to the env to accumulate things

kwladyka19:03:28

Do you see better way to achieve something similar?

wilkerlucio19:03:42

and in the ::p/wrap-parser plugin you can inject the collected things to the final response

wilkerlucio19:03:38

how are you using this? what your query looks like?

kwladyka19:03:20

for example this is wrong query because of the validation

(utils/request-eql [{[:shop/uuids #{"999"}] [:shops]}])

kwladyka19:03:41

this is good one

(utils/request-eql [{[:shop/uuids #{"00000000-0000-0000-0000-000000000000"}] [:shops]}])

kwladyka19:03:24

and I want to return in some way errors during processing of this wrong query

kwladyka19:03:33

{:logs [{:type :error, :code :validation, :cause ({:via [:shop/uuids :shop/uuid], :val “999”, :message “UUID has to be in valid format (for example 00000000-0000-0000-0000-000000000000).“} {:via [:shop/uuids], :val #{“999”}, :message “Has to be set of uuid (for example #{00000000-0000-0000-0000-000000000000}).“})}]}

wilkerlucio19:03:35

I personally don't use collections as inputs, I found that given then a name is usually a better option (if possible)

wilkerlucio19:03:56

just to understand your case better, how do you build this set to make this query? (whats the source for this list?)

kwladyka19:03:04

> I found that given then a name is usually a better option (if possible) What do you mean?

kwladyka19:03:18

this query postgresql

wilkerlucio19:03:45

I mean, before, when you build the query. I imagine there is some place from which you get this set of ids

kwladyka19:03:33

I can get all shops by

(utils/request-eql [{[:shop/uuids :all] [:shops]}])

kwladyka19:03:53

Maybe I will change this later to separate resolver, will see

wilkerlucio19:03:08

so, what I would do there is [{:shop/all [:shop/name]}]

kwladyka19:03:14

but still the issue is I don’t know how can I return good feedback why query failed

wilkerlucio19:03:18

because, lists tend be something else

wilkerlucio19:03:29

you are using it in a peculiar way

wilkerlucio19:03:53

idents should refer to a single entity, that's more a Fulcro premise, but a lot of Pathom is built upon this idea

kwladyka19:03:38

what if I will have :shop/all as real value in :shop map data? It will make a little confusion.

wilkerlucio19:03:50

so, instead of trying to send them by hand on an ident, you can have a resolver that returns all the ids (like: [{:shop/id 1} {:shop/id 2} {:shop/id 3} ...])

kwladyka19:03:15

This is why I was thinking about not use /all to avoid confusion it is a value of :shop

wilkerlucio19:03:19

@kwladyka this is a very open graph, you should be consistent about what properties mean, so having it as both would be wrong modeling

wilkerlucio19:03:28

but you can give other names, as long as they are consistent

wilkerlucio19:03:27

sorry, I don't have a lot of time to chat around this now, gotta get back to work, but I suggest you try to follow the things as documented, at least until you get a better feeling of pathom, the direction you are taking is just unknown and likely not well supported

kwladyka19:03:32

I am experimenting a little

kwladyka19:03:50

but so far I stuck on :logs so the feedback why query failed (for example data validation)

wilkerlucio19:03:45

you are kind of manually dealing with list processing in this approach, pathom has a bunch of helpers to deal with sequences (think it like jQuery), also, errors in pathom are made in a way to be tolerant as much as possible

wilkerlucio19:03:23

I feel like you are thinking like entities (which is very common, that's what happens almost everywhere else) but in Pathom it really helps if you try to get your head around properties (not entities) as the primary building block

wilkerlucio19:03:34

so, when you think about something failing, its not the entity, but each attribute

wilkerlucio19:03:45

each attribute may fail for a different reason, so you can get partial results

wilkerlucio19:03:34

so, in my view, its not about the :shop/id failing, its about the detail of it failing because the dependency :shop/id was invalid, so the error would be on the final attribute (`:shop/name` or something)`, not on the list processing

wilkerlucio19:03:47

I really gotta go now, but would be happy to chat more around this some other time

kwladyka19:03:09

I see. But it makes it hard for API client processing. The client has to understand when the value is an error and when expected value.

kwladyka19:03:29

Unless I miss something

kwladyka19:03:48

But what you are saying make sense

kwladyka19:03:24

Only I don’t see how to make this easy to parse by client and understand when the client can use reposne and when not

kwladyka22:03:16

so after all… are you saying throwing exceptions is the right way of returning validations errors in pathom?

kwladyka22:03:42

I can’t find good way of doing this in pathom

kwladyka22:03:10

hmm with exceptions way it can be much simpler in pathom

myguidingstar18:03:27

@wilkerlucio is it possible to apply the batch transformer to a dynamic resolver?

wilkerlucio18:03:53

I think it should be, but I didn't get to play with that on dynamic resolvers yet

myguidingstar18:03:15

you mean it's already supported? how?

wilkerlucio18:03:28

I just did a check, its not supported yet

wilkerlucio18:03:50

but its on my list, if you have a close case you need it, I can prioritize it

wilkerlucio18:03:50

its not a hard thing to add

myguidingstar18:03:16

I'm converting walkable to a dynamic resolver, it'll be much easier if it's supported

wilkerlucio18:03:01

ok, tomorrow I'll have some time, I'll try to add it, I think the basic support (like the previous ones does) should be strait forward to implement

wilkerlucio18:03:30

I also would like to support multi-depth batching in this new one, that may take more work

myguidingstar18:03:21

even when you can't implement it soon, please give me several lines of example code so I know what to expect when it's done

wilkerlucio18:03:28

hard to tell with certain without sitting and giving some thinking, but I think it will be pretty much like the current one, instead of getting a map as input, you will get a sequence of maps (the inputs to batch)

myguidingstar18:03:26

what is "multi-depth batching"? Say [{:a [{:b [:c]}]}], does it mean all the :c children from multiple :b branches?

wilkerlucio18:03:19

yeah, like: [{:people [{:person/family [:person/name]}]}]

wilkerlucio18:03:40

I had people asking about this before, not in the near future, but something to think about how to implement over time

wilkerlucio19:03:12

started looking into it, I found that batch already works for regular resolvers on reader3, the issue is that batch relies on caching, and dynamic resolvers don't support caching due to their complex input/output relations

wilkerlucio19:03:25

I'm starting to think about how we can go around that for dynamic resolvers

myguidingstar20:03:04

What kind of caching? I use p/cached in my dynamic resolver and it seems to work

wilkerlucio20:03:12

p/cached works, but batch relies on the standard resolver caching, which doesn't work for dynamic, the reason is, when we do a batch for regular resolvers, we get the batched result, and then cache each entry as if they were called separated, so when the processing continues and the user reach that same resolver, the batch will have the call cached, this way it goes fast

wilkerlucio20:03:08

but for dynamic resolvers that's much harder, given there is not well defined input to cache from a dynamic response (what is the input? what if the request was already composed from multiple things on the dynamic, how to isolate that part?)

myguidingstar20:03:47

Btw I've made progress on converting walkable to a dynamic resolver. The batching was done by leveraging p/cached that makes batched query from parent node :)

kwladyka19:03:06

How do you return errors (not exception ,but for example spec validations info) from resolvers?

yenda19:03:44

For a search resolver, should a partial name be rather an input or a parameter?

wilkerlucio19:03:25

I usually do as a param

yenda19:03:55

yeah it was my first intuition but the issue is that the search is not part of the response

yenda19:03:57

would that make sense to return it as a field then?

wilkerlucio19:03:20

if you need in on the response as well, yes, you can do that