ahyatt / emacs-websocket

A websocket implementation in elisp, for emacs.
GNU General Public License v2.0
324 stars 42 forks source link

make nowait optional #50

Closed yuya373 closed 7 years ago

yuya373 commented 7 years ago

Hi! I'm using this library in yuya373/emacs-slack: slack client for emacs and works almost perfectly. But when I lost network connection and get another network connection (eg. home wifi → train → office wifi) and trying reconnect by websocket-open, websocket-open hangs at this line.

I set :nowait t, it does not hangs and successfully connected. Is there any drawback to set :nowait t?

ahyatt commented 7 years ago

Hm, thanks for reporting this. I don't know why changing nowait would solve your problem - setting it to t should attempt to make the connection asynchronous, and I'm not immediately sure what effect that will have, although possibly it would return from that function while making the connection in the background - which isn't what the code expects. I'll have to experiment with this and get back to you.

yuya373 commented 7 years ago

@ahyatt Hi, thanks for reply.

which isn't what the code expects.

this means these handshaking processes?

ahyatt commented 7 years ago

Sorry for the delay in response, I was on vacation.

Yes, the handshaking process probably is not going to run. I tried this out and verified that indeed the process status, for me, was connect by the time we got to the handshaking, as opposed to the open it would have to be to succeed.

We could send the handshake when the status changes to open (as well as calling the on-open method the client may have passed in). Perfectly reasonable, but this requires some further changes. If you do that, please make sure the functional tests pass as well.

yuya373 commented 7 years ago

@ahyatt Hi! I moved to handshaking processes into sentinel lambda at https://github.com/ahyatt/emacs-websocket/pull/50/commits/f7f7ad6d45664ac872541dcf9fdaa37f75fa6136

I need another sleep to pass functional test(https://github.com/ahyatt/emacs-websocket/pull/50/commits/99d3b3e695bac577505371488e1081533851783d). this sleep is needed to pass test on master branch in my environment. 🤔

This is GNU Emacs 26.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.22.15)
 of 2017-08-12

below is output without sleep websocket-ready-state is connecting.

INSERT ❯ emacs -batch -Q -L . -l websocket-functional-test.el
Testing with local server
Opening the websocket
ws frame: "You said: 你好"
Error (websocket): in callback `on-message': error: "Test error (expected)"
ws frame: "You said: Hi after error!"
Testing with wss://echo.websocket.org
[DEBUG] websocket-ready-state: connecting
Debugger entered--Lisp error: (cl-assertion-failed ((eq 'open (websocket-ready-state wstest-ws)) nil))
  cl--assertion-failed((eq 'open (websocket-ready-state wstest-ws)))
  (or (eq 'open (progn "Access slot \"ready-state\" of `(websocket (:constructor nil) (:constructor websocket-inner-create))' struct CL-X." nil (or (progn nil (and (memq (type-of wstest-ws) cl-struct-websocket-tags) t)) (signal 'wrong-type-argument (list 'websocket wstest-ws))) (aref wstest-ws 1))) (cl--assertion-failed '(eq 'open (websocket-ready-state wstest-ws))))
  (progn (or (eq 'open (progn "Access slot \"ready-state\" of `(websocket (:constructor nil) (:constructor websocket-inner-create))' struct CL-X." nil (or (progn nil (and (memq (type-of wstest-ws) cl-struct-websocket-tags) t)) (signal 'wrong-type-argument (list 'websocket wstest-ws))) (aref wstest-ws 1))) (cl--assertion-failed '(eq 'open (websocket-ready-state wstest-ws)))) nil)
  (progn (message "Testing with wss://echo.websocket.org") (if (eq system-type 'windows-nt) (progn (message "Windows users must have gnutls DLLs in the emacs bin directory."))) (setq wstest-ws (websocket-open "wss://echo.websocket.org" :on-open (function (lambda (_websocket) (message "Websocket opened"))) :on-message (function (lambda (_websocket frame) (setq wstest-msgs (cons (websocket-frame-text frame) wstest-msgs)) (message "ws frame: %S" (websocket-frame-text frame)))) :on-close (function (lambda (_websocket) (message "Websocket closed") (setq wstest-closed t)))) wstest-msgs nil) (sleep-for 0.3) (progn (or (websocket-openp wstest-ws) (cl--assertion-failed '(websocket-openp wstest-ws))) nil) (message "[DEBUG] websocket-ready-state: %s" (progn "Access slot \"ready-state\" of `(websocket (:constructor nil) (:constructor websocket-inner-create))' struct CL-X." nil (or (progn nil (and (memq (type-of wstest-ws) cl-struct-websocket-tags) t)) (signal 'wrong-type-argument (list 'websocket wstest-ws))) (aref wstest-ws 1))) (progn (or (eq 'open (progn "Access slot \"ready-state\" of `(websocket (:constructor nil) (:constructor websocket-inner-create))' struct CL-X." nil (or (progn nil (and (memq (type-of wstest-ws) cl-struct-websocket-tags) t)) (signal 'wrong-type-argument (list 'websocket wstest-ws))) (aref wstest-ws 1))) (cl--assertion-failed '(eq 'open (websocket-ready-state wstest-ws)))) nil) (progn (or (null wstest-msgs) (cl--assertion-failed '(null wstest-msgs))) nil) (websocket-send-text wstest-ws "Hi!") (sleep-for 1) (progn (or (equal (car wstest-msgs) "Hi!") (cl--assertion-failed '(equal (car wstest-msgs) "Hi!"))) nil) (websocket-close wstest-ws))
  (if (>= (string-to-number (substring emacs-version 0 2)) 24) (progn (message "Testing with wss://echo.websocket.org") (if (eq system-type 'windows-nt) (progn (message "Windows users must have gnutls DLLs in the emacs bin directory."))) (setq wstest-ws (websocket-open "wss://echo.websocket.org" :on-open (function (lambda (_websocket) (message "Websocket opened"))) :on-message (function (lambda (_websocket frame) (setq wstest-msgs (cons (websocket-frame-text frame) wstest-msgs)) (message "ws frame: %S" (websocket-frame-text frame)))) :on-close (function (lambda (_websocket) (message "Websocket closed") (setq wstest-closed t)))) wstest-msgs nil) (sleep-for 0.3) (progn (or (websocket-openp wstest-ws) (cl--assertion-failed '(websocket-openp wstest-ws))) nil) (message "[DEBUG] websocket-ready-state: %s" (progn "Access slot \"ready-state\" of `(websocket (:constructor nil) (:constructor websocket-inner-create))' struct CL-X." nil (or (progn nil (and (memq (type-of wstest-ws) cl-struct-websocket-tags) t)) (signal 'wrong-type-argument (list 'websocket wstest-ws))) (aref wstest-ws 1))) (progn (or (eq 'open (progn "Access slot \"ready-state\" of `(websocket (:constructor nil) (:constructor websocket-inner-create))' struct CL-X." nil (or (progn nil (and (memq (type-of wstest-ws) cl-struct-websocket-tags) t)) (signal 'wrong-type-argument (list 'websocket wstest-ws))) (aref wstest-ws 1))) (cl--assertion-failed '(eq 'open (websocket-ready-state wstest-ws)))) nil) (progn (or (null wstest-msgs) (cl--assertion-failed '(null wstest-msgs))) nil) (websocket-send-text wstest-ws "Hi!") (sleep-for 1) (progn (or (equal (car wstest-msgs) "Hi!") (cl--assertion-failed '(equal (car wstest-msgs) "Hi!"))) nil) (websocket-close wstest-ws)))
  eval-buffer(#<buffer  *load*> nil "/home/yuya373/dotfiles/.emacs.d/el-get/websocket/websocket-functional-test.el" nil t)  ; Reading at buffer position 4379
  load-with-code-conversion("/home/yuya373/dotfiles/.emacs.d/el-get/websocket/websocket-functional-test.el" "/home/yuya373/dotfiles/.emacs.d/el-get/websocket/websocket-functional-test.el" nil t)
  load("/home/yuya373/dotfiles/.emacs.d/el-get/websocket/websocket-functional-test.el" nil t)
  command-line-1(("-L" "." "-l" "websocket-functional-test.el"))
  command-line()
  normal-top-level()
ahyatt commented 7 years ago

The change is looking good. Don't worry about the extra sleep, timing is different on each machine. I should make it a bit more robust. So just revert that change, please.

Also, can you also add some documentation to the function documentation string about your new argument?

yuya373 commented 7 years ago

Okay, I forget about extra sleep. I reverted change and added documentation.

ahyatt commented 7 years ago

Thanks for the change! I'll test it out a bit more and rework the documentation string a bit.