clj-commons / manifold

A compatibility layer for event-driven abstractions
1.02k stars 106 forks source link

Question: How to extract exception error from deferred? #38

Closed zentrope closed 9 years ago

zentrope commented 9 years ago

Using (http/websocket-connection req) I might get an error (in the form of an exception). How do I get it out so I can, say, print out what it was?

(let [stream @(http/websocket-connection)]
  (if (d/deferred? stream)
    (println (.getMessage (d/error-value stream (Exception. "Unknown."))))
    (do-something-with stream)))

That seems to work, but feels like I'm missing the point.

If I (.getMessage @stream), something locks up. Somehow a manifold.deferred.ErrorDeferred is not really a deferred? I don't know.

Also error-value is not in the documentation (which is marked as 0.1.1-SNAPSHOT).

I should probably be using d/loop and d/recur and d/catch and d/chain for processing messages, right?

ztellman commented 9 years ago

error-value exists as a minor optimization for internal use, the lack of documentation is intentional. Dereferencing a deferred in an error state will throw the exception, not return it.

There are a number of examples of how to handle the error case at http://ideolalia.com/aleph/literate.html#aleph.examples.websocket, please feel free to reopen the issue if you have any further questions.

zentrope commented 9 years ago

Thanks! Happy for the examples.

This:

(defn echo-handler
  [req]
  (if-let [socket (try
                    @(http/websocket-connection req)
                    (catch Exception e
                      nil))]
    (s/connect socket socket)
    non-websocket-request))

And then curl http://host:port/ws I don't get an exception thrown. Instead, I get a manifold.deferred.ErrorDeferred, which then throws an exception when I attempt to use it as a valid stream.

Here's my exact code:

(defn chat
  [req]
  (if-let [stream (try @(http/websocket-connection req) (catch Throwable _ nil))]
    (go (info "client connect")
        (loop []
          (when-let [msg @(stream/take! stream)]
            (info "msg>" (clojure.edn/read-string msg))
            (recur)))
        (info "client disconnect"))
    {:status 400
     :headers {"content-type" "plain/text"}
     :body "Invalid web-socket request."}))

Works great for an actual legitimate request, though.

If the request is legit, the (deref (http/websocket-connection ...)) returns a stream (rather than a deferred, which makes sense because deref is providing the value of the deferred when ready). But if the connection is invalid, the deref returns another derefable thing, as far as I can tell.

zentrope commented 9 years ago

Sorry. Can't re-open an issue if I didn't close it myself.

zentrope commented 9 years ago

(This should be in Aleph, I guess.)

Interesting. If I use the code I quoted above, two things happen:

I'm not sure I understand what's going on here:

https://github.com/ztellman/aleph/blob/master/src/aleph/http/server.clj#L509-L559

But is it possible that the first try (line 536) is creating a defer around the exception, and the catch' at line 554 is wrapping the first defer in another one?

Regardless, derefing the websocket constructor isn't throwing.

ztellman commented 9 years ago

Okay, it looks like you're testing whether a request is a websocket handshake by trying to establish a websocket connection. However, websocket-connection is only meant to be called on requests that you're sure should be a websocket handshake, and so anything which is not is treated as an exceptional condition, and a 400 status is returned if the handshake fails.

I can add a websocket-request? predicate to Aleph, but you should be able to write one yourself by checking the headers without too much trouble.

However, the double-dereference behavior you're seeing is a bug. Using catch will properly flatten nested errors, but it appears that catch' will not. As a workaround for now, I suggest using this:

(defn echo-handler
  [req]
  (if-let [socket @(d/catch
                             (http/websocket-connection req)
                             (fn [_] nil))]
    (s/connect socket socket)
    non-websocket-request))
zentrope commented 9 years ago

Thanks! I actually had something very similar to that earlier on. ;)

You're right about using headers to check for a proper connection type, etc, etc. I don't think anything like it needs to be a part of Aleph. The double-deferred was confusing, though. Good luck and thanks for taking a look!

ztellman commented 9 years ago

This is fixed as of https://github.com/ztellman/manifold/commit/2a7a6fab4f2a2bda9b914a3b3b6ace907efd2a33, I'll be adding a websocket-request? to Aleph in the next release as well.