Skip to content
This repository

Websocket Adjustments #955

Merged
merged 4 commits into from over 2 years ago

3 participants

Min RK Brian E. Granger Fernando Perez
Min RK
Owner

Two principal changes:

  1. alert client on failed and lost web socket connections

A long message is given if the connection fails within 1s, which assumes the connection did not succeed. Otherwise, it is a short 'connection closed unexpectedly'.

This also means that clients are notified on server termination (for better or worse).

  1. remove superfluous ws-hostname parameter from notebook

This made the notebook server artificially and unnecessarily brittle against tunneling and explicit hostname resolution. Now, the ws_url is defined based on the Origin of the request for the url, so it always matches the http[s] url. This means that it will follow the same tunnel, and the hostname will be already resolved. Resolving the hostname twice makes no sense at all unless the websockets are going to a different server than the http requests.

Implemented as a property, so it should still be easy to change for future cases where it might behave differently (e.g. websockets on a different host, or at a non-root url).

This is one approach to closing #952, but there might be better ways to do it (e.g. dialogs that are not base alert calls, which block window interaction).

added some commits October 30, 2011
Min RK remove superfluous ws-hostname parameter from notebook
This made the notebook server artificially and unnecessarily brittle to tunneling, and non-local hostname resolution.  The ws_url is now defined based on the Origin of the request.  This allows tunneled hosts and ports to preserve connections, as the ws_url always matches the one in use by the client.

Implemented as a property, to facilitate future cases where it might behave differently.
f00c904
Min RK alert client on failed and lost web socket connections
A long message is given if the connection fails within 1s.  Otherwise, it is a short 'connection closed unexpectedly'.

This also means that clients are notified on server termination.
81cfbe8
Brian E. Granger
Owner

@minrk: removing ws-hostname is fine. Originally, I thought we might need to run the ws stuff in a different process, but that didn't pan out.

Min RK
Owner

I changed the error message to be a jQuery dialog rather than a plain alert, so it's less obstructive.

Min RK
Owner

@ellisonbg - that makes sense. It should still be easy to change, in environments where the websocket host and/or port differ from the rest.

Fernando Perez
Owner

Mh, I tried running a notebook with this branch on and I got this traceback:

ERROR:root:Uncaught exception POST /kernels?notebook=69f0f1e7-5cbf-4d0c-b7fe-7807e1023886 (128.32.52.133)
HTTPRequest(protocol='https', host='longs.berkeley.edu:9999', method='POST', uri='/kernels?notebook=69f0f1e7-5cbf-4d0c-b7fe-7807e1023886', version='HTTP/1.1', remote_ip='128.32.52.133', body='', headers={'Referer': 'https://longs.berkeley.edu:9999/69f0f1e7-5cbf-4d0c-b7fe-7807e1023886', 'Content-Length': '0', 'Accept-Language': 'en-us,en;q=0.5', 'Accept-Encoding': 'gzip, deflate', 'Host': 'longs.berkeley.edu:9999', 'Accept': 'application/json, text/javascript, */*; q=0.01', 'User-Agent': 'Mozilla/5.0 (X11; Linux x86_64; rv:7.0.1) Gecko/20100101 Firefox/7.0.1', 'Accept-Charset': 'UTF-8,*', 'Connection': 'keep-alive', 'X-Requested-With': 'XMLHttpRequest', 'Pragma': 'no-cache', 'Cache-Control': 'no-cache', 'Cookie': '__utma=245566491.1143703340.1319585252.1319585252.1320105682.2; __utmz=245566491.1319585252.1.1.utmcsr=(direct)|utmccn=(direct)|utmcmd=(none); username=YWUyZGI4NjctNDQzMS00ZGIxLWI1MDItNjkwNDk2MzJmNGY1|1320968643|33efb552dd2899f965dbcf4deb1d81d612e3a06f'})
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.6/site-packages/tornado-2.1-py2.6.egg/tornado/web.py", line 954, in _execute
    getattr(self, self.request.method.lower())(*args, **kwargs)
  File "/home/fperez/usr/local/lib/python2.6/site-packages/tornado-2.1-py2.6.egg/tornado/web.py", line 1667, in wrapper
    return method(self, *args, **kwargs)
  File "/home/fperez/usr/lib/python2.6/site-packages/IPython/frontend/html/notebook/handlers.py", line 232, in post
    data = {'ws_url':self.ws_url,'kernel_id':kernel_id}
  File "/home/fperez/usr/lib/python2.6/site-packages/IPython/frontend/html/notebook/handlers.py", line 150, in ws_url
    return self.request.headers.get('Origin').replace('http', 'ws', 1)
AttributeError: 'NoneType' object has no attribute 'replace'
ERROR:root:500 POST /kernels?notebook=69f0f1e7-5cbf-4d0c-b7fe-7807e1023886 (128.32.52.133) 39.75ms

I can't actually execute any cells. Do you get the same? Note that the traceback occurs on notebook connection, before any attempt to even execute code.

Min RK
Owner

Apparently Firefox doesn't send the Origin header, so I changed it to write protocol / host directly from the request. This will still work for tunnels, but I think it probably won't work if the notebook server is behind a reverse-proxy (e.g. if you have apache set up to forward http://server/ipnb to http://localhost:8888).

Fernando Perez
Owner

Great, tested and merging now, everything seems to work. Not having to pass the websocket argument is nice.

Fernando Perez fperez merged commit f6b3d8f into from November 10, 2011
Fernando Perez fperez closed this November 10, 2011
Fernando Perez fperez referenced this pull request from a commit January 10, 2012
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 4 unique commits by 1 author.

Oct 30, 2011
Min RK remove superfluous ws-hostname parameter from notebook
This made the notebook server artificially and unnecessarily brittle to tunneling, and non-local hostname resolution.  The ws_url is now defined based on the Origin of the request.  This allows tunneled hosts and ports to preserve connections, as the ws_url always matches the one in use by the client.

Implemented as a property, to facilitate future cases where it might behave differently.
f00c904
Min RK alert client on failed and lost web socket connections
A long message is given if the connection fails within 1s.  Otherwise, it is a short 'connection closed unexpectedly'.

This also means that clients are notified on server termination.
81cfbe8
Oct 31, 2011
Min RK use jQuery dialog instead of alert() b7803b5
Nov 10, 2011
Min RK don't use Origin header to determine ws_url bc78497
This page is out of date. Refresh to see the latest.
15  IPython/frontend/html/notebook/handlers.py
@@ -139,7 +139,16 @@ def read_only(self):
139 139
                 return True
140 140
         else:
141 141
             return False
  142
+    
  143
+    @property
  144
+    def ws_url(self):
  145
+        """websocket url matching the current request
142 146
         
  147
+        turns http[s]://host[:port] into
  148
+                ws[s]://host[:port]
  149
+        """
  150
+        proto = self.request.protocol.replace('http', 'ws')
  151
+        return "%s://%s" % (proto, self.request.host)
143 152
 
144 153
 
145 154
 class ProjectDashboardHandler(AuthenticatedHandler):
@@ -221,8 +230,7 @@ def post(self):
221 230
         km = self.application.kernel_manager
222 231
         notebook_id = self.get_argument('notebook', default=None)
223 232
         kernel_id = km.start_kernel(notebook_id)
224  
-        ws_url = self.application.ipython_app.get_ws_url()
225  
-        data = {'ws_url':ws_url,'kernel_id':kernel_id}
  233
+        data = {'ws_url':self.ws_url,'kernel_id':kernel_id}
226 234
         self.set_header('Location', '/'+kernel_id)
227 235
         self.finish(jsonapi.dumps(data))
228 236
 
@@ -249,8 +257,7 @@ def post(self, kernel_id, action):
249 257
             self.set_status(204)
250 258
         if action == 'restart':
251 259
             new_kernel_id = km.restart_kernel(kernel_id)
252  
-            ws_url = self.application.ipython_app.get_ws_url()
253  
-            data = {'ws_url':ws_url,'kernel_id':new_kernel_id}
  260
+            data = {'ws_url':self.ws_url,'kernel_id':new_kernel_id}
254 261
             self.set_header('Location', '/'+new_kernel_id)
255 262
             self.write(jsonapi.dumps(data))
256 263
         self.finish()
18  IPython/frontend/html/notebook/notebookapp.py
@@ -147,7 +147,6 @@ def __init__(self, ipython_app, kernel_manager, notebook_manager, log):
147 147
     'port': 'NotebookApp.port',
148 148
     'keyfile': 'NotebookApp.keyfile',
149 149
     'certfile': 'NotebookApp.certfile',
150  
-    'ws-hostname': 'NotebookApp.ws_hostname',
151 150
     'notebook-dir': 'NotebookManager.notebook_dir',
152 151
 })
153 152
 
@@ -155,7 +154,7 @@ def __init__(self, ipython_app, kernel_manager, notebook_manager, log):
155 154
 # multi-kernel evironment:
156 155
 aliases.pop('f', None)
157 156
 
158  
-notebook_aliases = [u'port', u'ip', u'keyfile', u'certfile', u'ws-hostname',
  157
+notebook_aliases = [u'port', u'ip', u'keyfile', u'certfile',
159 158
                     u'notebook-dir']
160 159
 
161 160
 #-----------------------------------------------------------------------------
@@ -200,13 +199,6 @@ def _ip_changed(self, name, old, new):
200 199
         help="The port the notebook server will listen on."
201 200
     )
202 201
 
203  
-    ws_hostname = Unicode(LOCALHOST, config=True,
204  
-        help="""The FQDN or IP for WebSocket connections. The default will work
205  
-                fine when the server is listening on localhost, but this needs to
206  
-                be set if the ip option is used. It will be used as the hostname part
207  
-                of the WebSocket url: ws://hostname/path."""
208  
-    )
209  
-
210 202
     certfile = Unicode(u'', config=True, 
211 203
         help="""The full path to an SSL/TLS certificate file."""
212 204
     )
@@ -226,14 +218,6 @@ def _ip_changed(self, name, old, new):
226 218
         help="Whether to prevent editing/execution of notebooks."
227 219
     )
228 220
 
229  
-    def get_ws_url(self):
230  
-        """Return the WebSocket URL for this server."""
231  
-        if self.certfile:
232  
-            prefix = u'wss://'
233  
-        else:
234  
-            prefix = u'ws://'
235  
-        return prefix + self.ws_hostname + u':' + unicode(self.port)
236  
-
237 221
     def parse_command_line(self, argv=None):
238 222
         super(NotebookApp, self).parse_command_line(argv)
239 223
         if argv is None:
59  IPython/frontend/html/notebook/static/js/kernel.js
@@ -27,7 +27,7 @@ var IPython = (function (IPython) {
27 27
         } else if (typeof(MozWebSocket) !== 'undefined') {
28 28
             this.WebSocket = MozWebSocket
29 29
         } else {
30  
-            alert('Your browser does not have WebSocket support, please try Chrome, Safari or Firefox 6. Firefox 4 and 5 are also supported by you have to enable WebSockets in about:config.');
  30
+            alert('Your browser does not have WebSocket support, please try Chrome, Safari or Firefox 6. Firefox 4 and 5 are also supported by you have to enable WebSockets in about:config.');
31 31
         };
32 32
     };
33 33
 
@@ -87,8 +87,37 @@ var IPython = (function (IPython) {
87 87
         IPython.kernel_status_widget.status_idle();
88 88
     };
89 89
 
  90
+    Kernel.prototype._websocket_closed = function(ws_url, early){
  91
+        var msg;
  92
+        var parent_item = $('body');
  93
+        if (early) {
  94
+            msg = "Websocket connection to " + ws_url + " could not be established.<br/>" +
  95
+            " You will NOT be able to run code.<br/>" +
  96
+            " Your browser may not be compatible with the websocket version in the server," +
  97
+            " or if the url does not look right, there could be an error in the" +
  98
+            " server's configuration."
  99
+        } else {
  100
+            msg = "Websocket connection closed unexpectedly.<br/>" +
  101
+            " The kernel will no longer be responsive."
  102
+        }
  103
+        var dialog = $('<div/>');
  104
+        dialog.html(msg);
  105
+        parent_item.append(dialog);
  106
+        dialog.dialog({
  107
+            resizable: false,
  108
+            modal: true,
  109
+            title: "Websocket closed",
  110
+            buttons : {
  111
+                "Okay": function () {
  112
+                    $(this).dialog('close');
  113
+                }
  114
+            }
  115
+        });
  116
+        
  117
+    }
90 118
 
91 119
     Kernel.prototype.start_channels = function () {
  120
+        var that = this;
92 121
         this.stop_channels();
93 122
         var ws_url = this.ws_url + this.kernel_url;
94 123
         console.log("Starting WS:", ws_url);
@@ -97,17 +126,45 @@ var IPython = (function (IPython) {
97 126
         send_cookie = function(){
98 127
             this.send(document.cookie);
99 128
         }
  129
+        var already_called_onclose = false; // only alert once
  130
+        ws_closed_early = function(evt){
  131
+            if (already_called_onclose){
  132
+                return;
  133
+            }
  134
+            already_called_onclose = true;
  135
+            if ( ! evt.wasClean ){
  136
+                that._websocket_closed(ws_url, true);
  137
+            }
  138
+        }
  139
+        ws_closed_late = function(evt){
  140
+            if (already_called_onclose){
  141
+                return;
  142
+            }
  143
+            already_called_onclose = true;
  144
+            if ( ! evt.wasClean ){
  145
+                that._websocket_closed(ws_url, false);
  146
+            }
  147
+        }
100 148
         this.shell_channel.onopen = send_cookie;
  149
+        this.shell_channel.onclose = ws_closed_early;
101 150
         this.iopub_channel.onopen = send_cookie;
  151
+        this.iopub_channel.onclose = ws_closed_early;
  152
+        // switch from early-close to late-close message after 1s
  153
+        setTimeout(function(){
  154
+            that.shell_channel.onclose = ws_closed_late;
  155
+            that.iopub_channel.onclose = ws_closed_late;
  156
+        }, 1000);
102 157
     };
103 158
 
104 159
 
105 160
     Kernel.prototype.stop_channels = function () {
106 161
         if (this.shell_channel !== null) {
  162
+            this.shell_channel.onclose = function (evt) {null};
107 163
             this.shell_channel.close();
108 164
             this.shell_channel = null;
109 165
         };
110 166
         if (this.iopub_channel !== null) {
  167
+            this.iopub_channel.onclose = function (evt) {null};
111 168
             this.iopub_channel.close();
112 169
             this.iopub_channel = null;
113 170
         };
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.