How can I return a representation with an error response?

[aviflax]

For example, in the case of a malformed request, the function supplied as the value for :malformed-request? can only return a boolean value. But I want to specify the error response representation. Is there some way to do this?

[cmiles74]

I have to think about this one a little bit. I've run into a place where I wanted to do this as well, it seems like something we should be able to do. I'll get an enhancement in for this soon.

Thanks for the feedback. :)

[cmiles74]

I have added the ability to pass a response map to the "halt-resource" function. This will let you provide a body (and even appropriate headers) when you are returning a specific status code.

(def test-routes-webmachine
  {...routes here...
   ["*"] (halt-resource 404 {:body (str "Dude, bogus page!")})})

There might be other places where you'd like to return a specific status code, you can always add the ":status" key to your response map to get Bishop to produce that specific response. At the end of the day, the library is just merging these response maps together.

[aviflax]

Thanks Christopher! I appreciate the rapid and thoughtful response.

Unfortunately, I don’t quite understand how to use this new feature in the particular use cases I had in mind, wherein I’m specifying callbacks in my resource which check whether a resource exists, or is malformed, etc.

For example, I might implement this in a resource’s second map:

{:malformed-request? (fn [req] (is-malformed? req))}

so how can I use halt-resource in this context to return an error representation? (Specifically, one which is specific to the specific case at hand. In other words, I want to return a different error representation for a missing resource versus a malformed one, versus an internal server error, etc.)

[cmiles74]

I see what you are saying... Right now when these callbacks are called, the value is expected to be either truthy or falsey and in almost every case the result is discarded. This code is modeled on the Ruby Webmachine port and I decided to stay close to how that was working to keep things easier on myself.

(defn b9b
  "Test if this request is Malformed"
  [resource request response state]
  (decide #(apply-callback request resource :malformed-request?)
          true
          (response-code 400 request response state :b9b)
          #(b8 resource request response (assoc state :b9b false))))

It's easy to see why you'd want to include some sort of body, especially if you were validating some incoming data structure. Perhaps the result of the callback can simply be saved and, if it's a map, merged into the outgoing response.

[aviflax]

@cmiles74 that sounds good to me!

Just to be clear, my code could look something like this:

{:malformed-request?
 (fn [req]
   (when (is-malformed? req)
     {:headers {"Content-Type" "text/plain"}
      :body "That’s an awful request. What were you thinking?"}))}

And Bishop would halt processing the request, and return a 400 response with my specified response values?

If so, that would be great.

Also, this would apply to :resource-exists?, :is-authorized?, etc., right?

[cmiles74]

Yep, that's what I'm thinking. :) It'd give you room to customize the error response but that's about it. The same mechanism is used for of many of the handlers that return a status code (including :resource-exists?) so it'd work for most of them.

The :is-authorized? handler is a little special, if the client isn't currently authorized the function will return a String that's used to populate the "WWW-Authenticate" header (see here). There are a couple that work this way, but I think the same mechanism can be used. In the case of :is-authorized?, if a String is returned the current behavior is used. If a map is provided, it will look for a key called :www-authenticate and the rest of the map will be used as the response.

In terms of making the change, I think I'll have to get the change in place and see how the unit tests work out. Thinking about it, I don't foresee any big problems. This seems like a big enough change that maybe I'll create a branch and get something working fairly soon.

[aviflax]

Sounds great, let me know if I can help!

[cmiles74]

I have created a new branch with the changes so far. Handling a callback that wants to return a response map instead of a "true" was pretty easy, handling "false" a little trickier. I have made this compromise, callback functions may return one of the following:

• true
• false
• A response map, implies true
• A sequence, the first item being either true or false and the second item contains the response map

The :is-authorized callback is a weird one, we're looking for "true", "false" or a String. I haven't made it to that handler yet but if you wanted to return a response along with the error, I think it makes sense to set the "WWW-Authenticate" header in the callback's return value.

(fn [req]
  [false
   {:headers {"www-authenticate" "Basic realm=\"Secure Area\""
              "content-type" "text/plain"}
    :body "Seriously, you thought you could just waltz right in?"}])

In the case of callbacks like :malformed-request?, where the affirmative response indicates that the error status should be returned, we can just return the response map; it's the same as true and much nicer looking.

(fn [req]
   (when (is-malformed? req)
     {:headers {"Content-Type" "text/plain"}
      :body "That’s an awful request. What were you thinking?"}))

I'm not totally in love with this solution, it seems a little clunky to me. Still, I like it better than adding a new key to the response map (easy to forget). Enough of this change is coded up than I know it'll work across the board, I hope to complete this work soon as I need this functionality myself on another project.

[cmiles74]

I have finished coding up this feature, it's available in the 1.1.0 release. :-) More information on which callbacks can return a response can be found in the wiki.

[aviflax]

Excellent, I’ll give it a try, thanks!

[aviflax]

Christopher, this is working well for me in most cases, thanks!

One issue: this doesn’t seem to have been implemented for :resource-exists? but I need it to be. Is there a specific reason this wasn’t implemented for that callback, or was it just overlooked? Might it be possible to add support for this behavior to that callback?

For reference, here’s my callback:

     :resource-exists?
     (fn [req]
       (if (global-term-exists? (get-in req [:path-info :term-name]))
           true
           [false {:body "The requested resource does not exist."}]))

in my project, using 1.1.0, it appears that the false case, since it’s a vector, is seen as truthy, and therefore Bishop thinks my resource exists, when in fact it does not.

Thanks!

[cmiles74]

Ouch! It looks like I just completely missed it. I'll get that fixed later today. :-$

[cmiles74]

Okay, I added the ability to return a response body from the :resource-exists? handler, this is in release 1.1.2. This one wasn't included when I did my last change because it doesn't directly return a response code (:resource-exists? is called in node G7 and if it returns false then the request moved to node G8, etc.)

Also, take a look at some of the other handlers functions: :previously-existed?, :moved-permanently? and :moved-temporarily. Both :moved-permanently? and :moved-temporarily may return a new URI that points to the new location of the resource, these two callbacks are invoked if :previously-existed? returns true.

[aviflax]

Thanks, I'll check it out!

[aviflax]

Using 1.1.2, I’m having trouble getting :resource-exists? to return a response body.

My handler looks like this:

     :resource-exists?
     (fn [req]
       (if (global-term-exists? (get-in req [:path-info :term-name]))
           true
           [false {:status 404
                   :headers {"Content-Type" "text/plain"}
                   :body "The requested resource does not exist."}]))

My test:

$ curl -i -X GET http://localhost:3000/terms/foo
HTTP/1.1 404 Not Found
Date: Fri, 20 Jul 2012 02:54:32 GMT
Vary: accept-charset, accept
Content-Type: application/xml;charset=utf-8
Content-Length: 0
Server: Jetty(7.6.1.v20120215)

[cmiles74]

I take a look at this later this morning. And, of course, add another test case. :-P

[cmiles74]

I'm having trouble reproducing this one. I know it's a long shot, but could you try clearing out any old versions of the library? There was a brief period where I accidentally uploaded 1.1.1 with the name 1.1.2. It's unlikely you actually managed to get a hold of it, but you never know.

rm -rf ~/.m2/repository/tnrglobal/bishop/1.1.2/

Also, clear out the built artifact as well. That'd be the "target" folder with Leiningen 2 or the "deps" folder with the older version.

I have added another test case where the :resource-exists? handler also returns a status code and it's passing. I pasted your response into a test project and it works okay there, I'm hoping that some library/versioning thing is the source of this issue. Let me know if it's not and I'll head back to the drawing board.

[aviflax]

OK, I cleared out both the locations you recommended, and it’s working now. Thanks!

Unfortunately, most of my tests are now failing… somehow this has revived the problem I described in issue #12. I don’t know what’s going on.

[cmiles74]

I still catching up with you. :P I'm working on a bug I found while looking at issue #9, it's typo (symbol instead of variable). I'm going to push that out in the next 30 minutes... Are you on Freenode IRC? I can ping you when it's out.

Miles

Avi Flax wrote:

OK, I cleared out both the locations you recommended, and it’s working now. Thanks!

Unfortunately, most of my tests are now failing… somehow this has revived the problem I described in issue #12. I don’t know what’s going on.

---

Reply to this email directly or view it on GitHub: https://github.com/tnr-global/bishop/issues/7#issuecomment-7132966

[aviflax]

I did upgrade to Lein 2 last night… maybe that’s part of the problem. It keeps saying “all namespaces already :aot compiled” which makes me nervous…

I’ll see if I can get on Freenode.

[aviflax]

I’m trying to contact you on Freenode but not getting any responses… no big deal.

Anyway, I’m now doing this:

1. Change the version of Bishop in my project.clj
2. rm -Rf /Users/avi/.m2/repository/tnrglobal; rm -Rf target; lein run
3. lein test
4. repeat

Everything is fine with 1.1.0, but with 1.1.1 and 1.1.2 I get the exception described in #12 for all my POST tests. Have you maybe not tested the :process-post handler with a form-post?

I’m using some middleware too:

(def ring-handler
  "this is a var so it can be used by lein-ring"
  (-> (bishop/handler routes)
      (wrap-resource ,,, "public")
      (wrap-file-info ,,,)
      (wrap-params ,,,)
      (wrap-multipart-params ,,,)))

[cmiles74]

Hi Avi,

I think I have this fixed with the 1.1.3 release.

https://clojars.org/tnrglobal/bishop/versions/1.1.3

I introduced a typo with 1.1.2 where I used ":status" instead of "status". :-$ Sorry about that.

BTW, my nick is "cmiles74" and I lurk in #clojure.

[aviflax]

Fixed! Thanks!