Introduce an :error state for client resource

[easye]

If socket I/O burps a condition as happens on socket timeout, set the
state of the client to :error in case anyone retains a reference to it
from our copy of the collection Hunchentoot sockets that we have
upgraded to the Websocket protocol.

#24

[joaotavora]

Not very happy with this, but let's go step by step: why are you saving a reference to the wesocket-client instance?

[easye]

I am using the reference to websocket-client to write an continuous stream of messages only to that client in another thread.

[joaotavora]

OK. Why aren't you calling with-new-client-for-resource in that other thread?

[easye]

with-new-client-for-resource has already been invoked by the time I create the new client.

Should I call it more than once for a new client on the given resource? I might well have misunderstood the API.

[joaotavora]

Should I call it more than once for a new client on the given resource? I might well have misunderstood the API.

No no, that part is just fine :-)

with-new-client-for-resource has already been invoked by the time I create the new client.

But why? Why aren't you creating it in the thread that is using it? If you're starting the thread with a lambda, it's a question of changing with-new-client-for-resource to inside that lambda.

[joaotavora]

@easye , I'm sorry I'm confusing you and myself.

The with-new-client-for-resource isn't even API, so never mind my last few comments.

[joaotavora]

OK, now that I've re-read the cde, my previous questions didn't make any sense.

What is preventing you from detecting the disconnect in client-disconnected? Is it that you wish to know the reason for the disconnect?

[joaotavora]

In your code, BTW, you seem to be setting :error even if it was a total innocent disconnect.

[easye]

In your code, BTW, you seem to be setting :error even if it was a total innocent disconnect.

That would be wrong on my part. I guess it is not an :error: so much as a :hunchentoot-signalled-a-condition, huh?

[joaotavora]

That would be wrong on my part. I guess it is not an :error: so much as a :hunchentoot-signalled-a-condition, huh?

Yep, and we should save the condition for maximum information.

[joaotavora]

What about this?

diff --git a/hunchensocket.lisp b/hunchensocket.lisp
index 623db08..3d3daa2 100644
--- a/hunchensocket.lisp
+++ b/hunchensocket.lisp
@@ -46,7 +46,11 @@
    (write-lock :initform (make-lock))
    (state      :initform :disconnected)
    (pending-fragments :initform nil)
-   (pending-opcode    :initform nil)))
+   (pending-opcode    :initform nil)
+   (disconnect-condition
+    :reader disconnect-condition
+    :documentation "Unbound if client is still connected, nil on
+normal termination, a condition on abnormal termination.")))
 
 (defmethod initialize-instance :after ((client websocket-client)
                                        &key &allow-other-keys)
@@ -171,7 +175,16 @@ format control and arguments."
              (push client clients))
            (setf (slot-value client 'state) :connected)
            (client-connected resource client)
-           (funcall fn))
+           (setf (slot-value client 'disconnect-condition)
+                 (catch 'websocket-done
+                   (handler-bind
+                       ((error #'(lambda (e)
+                                   (maybe-invoke-debugger e)
+                                   (log-message* :error "Error: ~a" e)
+                                   (throw 'websocket-done e))))
+                     (funcall fn)
+                     ;; on normal termination, return nil
+                     nil))))
       (bt:with-lock-held (lock)
         (with-slots (write-lock) client
           (bt:with-lock-held (write-lock)
@@ -511,13 +524,7 @@ payloads."
           #+lispworks
           (setf (stream:stream-read-timeout stream) timeout
                 (stream:stream-write-timeout stream) timeout)
-          
-          (catch 'websocket-done
-            (handler-bind ((error #'(lambda (e)
-                                      (maybe-invoke-debugger e)
-                                      (log-message* :error "Error: ~a" e)
-                                      (throw 'websocket-done nil))))
-              (read-handle-loop resource client))))))))
+          (read-handle-loop resource client))))))
 
 (defmethod handle-handshake ((acceptor websocket-acceptor) request reply)
   "Analyse REQUEST for WebSocket handshake.
diff --git a/package.lisp b/package.lisp
index 46b9535..1a7cef2 100644
--- a/package.lisp
+++ b/package.lisp
@@ -29,6 +29,7 @@
    ;;
    #:client-connected
    #:client-disconnected
+   #:disconnect-condition
    
    ;; receiving and sending messages
    ;; 

[joaotavora]

@easye, any comment on my proposal? Does it fix this particular problem?

[hanshuebner]

Is this still relevant?