I would go with 1, that way you don't have to update the Vase docs everytime the Pedestal docs changes

1, with a short roadmap doc.

To follow up on my earlier issue, my fairly new pedestal service started throwing exceptions when starting the REPL, complaining that No matching ctor found for class io.pedestal.http.ring_middlewares$response_fn_adapter$fn__11673. After numerous lein clean, deleting and re-loading dependencies in ~/.m2, and other suggestions, I was still getting the error, but today it stopped happening.

@manutter51 I hate when bugs just go away on their own. That means they can come back on their own, too.

I didn’t have time this morning to fully test it, but the thing that seems to have done the trick is moving the session configuration to the service map in service.clj. I thought I had done that earlier, but I realized I had added it to the map in server.clj that run-dev merges with the main service map.

moving the session setting to the right map seems to have fixed the issue

though I have no clue why that particular cause would have that particular effect.

Is the only way to exit the interceptor chain by throwing an exception? For example, I have 3 interceptors for my route and interceptor1 decides that the request should not be completed right now (maybe it was an invalid request, user not authorized, the server has other processing duties, etc.). Does interceptor1 have to throw an exception in order to stop subsequent interceptors from running, or is there another way?

I believe I found my answer 🙂 http://pedestal.io/api/pedestal.interceptor/io.pedestal.interceptor.chain.html#var-terminate

@kenny you can also have your interceptor return a response

Once an interceptor returns a response, the execution of downstream interceptor :enter fns is terminated

whoops

@kenny not return a response, assoc a response onto the context

so to state it correctly, once an interceptor assoc’s a response to the context, the execution of downstream interceptor :enter fns is terminated

Ah gotcha. Unless I missed it, I don't believe that was mentioned in the docs.

Is the :leave called of subsequent interceptors?

so the :leave fn is called for all interceptors that were already executed

Makes sense.

Hmm, I think you’re right about the docs (http://pedestal.io/reference/interceptors), they don’t mention this.

Yeah. I'd think it would go under "Interceptor Return Values"

Not sure if you'd ever want to do this but is it possible to terminate the :leave execution as well?

Wait I don't think associng a :response onto the context stops the interceptor flow. I just tried it and my second interceptor got called

hmm, I do this all the time

The second interceptor does not get called if I wrap the context with terminate.

Yeah the second interceptor is definitely getting called even though the context has a :response assoc'ed on it.

I think that makes sense though.

Explicit termination is much better than implicit. Usually 😉

The interceptor after the one which assoc’d the response?

Yes

(def echo
  {:name :echo
   :enter
         (fn [context]
           (clojure.pprint/pprint context)
           (let [request (:request context)
                 response (ok context)]
             (assoc context :response response)))})

(def echo2
  {:name  :echo2
   :enter (fn [context]
            (println "echo2")
            context)})

(def routes
  (route/expand-routes
    #{["/foo" :post [echo echo2] :route-name :foo]}))

echo2 is printed

hrmm, interesting

If instead echo calls terminate:

(def echo
  {:name :echo
   :enter
         (fn [context]
           (clojure.pprint/pprint context)
           (let [request (:request context)
                 response (ok context)]
             (chain/terminate (assoc context :response response))))})

I’m going to play with this because it doesn’t behave how I expected. Perhaps things have changed!

You may be getting unnecessary database load if the chain isn't actually terminating 🙂

@kenny are you using a custom chain provider?

or a default setup using lein new pedestal-service

This is the ns I'm messing with: http://pastebin.com/eMTkYEwu

@kenny, so this example (https://gist.github.com/ddeaguiar/74eed4784848e49fa83179a2086aa9ea) behaves as I described. Because the :io.pedestal.interceptor.chain/terminators coll on the context has a single predicate which is set by the default servlet chain provider. https://github.com/pedestal/pedestal/blob/41337e41ae773f3c17baeb1c244be13ddf6c28db/service/src/io/pedestal/http/impl/servlet_interceptor.clj#L231

Your example appears to use the default servlet chain provider as well

In my example, when you request you’ll see this in the logs

16:01:31.660 [qtp2141354770-51] INFO  terminator.service - {:msg "In Terminator", :line 23}                                  │
16:01:31.663 [qtp2141354770-51] INFO  terminator.service - {:terminators (#function[io.pedestal.http.impl.servlet-interceptor│
16:01:31.663 [qtp2141354770-51] DEBUG io.pedestal.interceptor.chain - {:in check-terminators, :terminate? true, :execution-id│
16:01:31.664 [qtp2141354770-51] DEBUG io.pedestal.interceptor.chain - {:in leave-all, :execution-id 19, :line 239}           │
16:01:31.666 [qtp2141354770-51] DEBUG io.pedestal.interceptor.chain - {:interceptor :terminator.service/terminator, :stage :l│
16:01:31.666 [qtp2141354770-51] INFO  terminator.service - {:msg "Leaving Terminator", :line 29}

I added the following line to my logback.xml file to log what the interceptor chain is doing:

<logger name="io.pedestal.interceptor.chain" level="DEBUG" />

I understand why

btw, I also recommend using fully qualified symbols in your routes

I copied this function from the guide.

(defn response [status body & {:as headers}]
  {:status status :body body :headers headers})

(ring.util.response/response? (response 200 "foo"))
=> false

Instead you need to do this:

(defn response [status body & {:as headers}]
  {:status status :body body :headers (or headers {})})

BTW, I really appreciate you asking this question. I’ve got a much deeper understanding of chain providers now!

Once you do that the interceptor chain is terminated when a :response is assoc'ed onto the context.

Same here 🙂

hrm I think Pedestal needs better response checking

like if something’s assoc’d to the context as :response but it’s not a valid response, then it should at least log a warning

So, everything that drives Pedestal can be found in the context

If you create a custom chain provider and don’t need terminators, then don’t add io.pedestal.interceptor.chain/terminators to the context

For example, the Fast Pedestal example creates an initial context https://github.com/pedestal/pedestal/blob/master/samples/fast-pedestal/src/fast_pedestal/fasterjetty_service.clj#L67

Notice how it lacks some keys

That’s part of a custom chain provider bound to direct-jetty-provider

So the nuance is, if you’re using a the default servlet chain provider, or another chain provider which terminates when a response is assoc’d to the context, then you don’t need to call terminate

I’ll add a quote from ohpauleez > please don't terminate unless the situation is dire

meaning, don’t explicitly call terminate

Really the only time you'd want to terminate is after you have returned a response.

and that’s handled for you

Right

So another thing to remember is that handlers are interceptors

So all of this makes sense, in retrospect

but we always put handlers at the end of the interceptor chain

I’ve actually never tried to put them anywhere else

Not sure if it'd ever make sense to put them anywhere else

exactly

It doesn’t. Why I’ve never done it

hehe, this was a great rabbit hole!

On a side note, this came up because I was considering things like authentication. It's surprising that by default the only Pedestal authentication library geheimtur uses exceptions for control flow: https://github.com/propan/geheimtur/blob/master/src/geheimtur/interceptor.clj#L14

other auth impls do similar things

Take buddy-auth as an example

But it is terrible for perf

https://github.com/funcool/buddy-auth

Just because other libs do it doesn't mean it was the right decision. IMO exceptions should only be used for exceptional things, not logic.

which, btw, I created a sample demonstrating it’s usage: https://github.com/pedestal/pedestal/pull/503

https://github.com/pedestal/pedestal/tree/master/samples/fast-pedestal > Avoid throwing exceptions (especially in interceptors)

yeah, I tend to agree with you

Also relevant: https://shipilev.net/blog/2014/exceptional-performance/

One of my fav articles on exception performance.

So, for example, with buddy-auth, you don’t have to throw, you could just return a response as well

My sample throws because that’s the default usage

Right but because they have throwing as a default usage it hints others that the use of exceptions is acceptable for control flow.

true

So when I write interceptors for things like validation, I short-circuit by returning a response

And although it may work, it certainly isn't performant and there loads of articles about why exceptions shouldn't be used for that purpose.

Yeah, that's exactly what I would do.

Anyway, the docs definitely need to be updated.

Thanks for the article link. I’ll check that out later

It's long but I think it's valuable knowledge 🙂

I'll open an issue in pedestal-docs. If I have time this weekend I'll submit a PR.

Ah, looks like you already did 🙂

hehe yeah. Feel free to pick it up