Skip to content

Commit

Permalink
Fix packages of exceptions catched by gateway manager
Browse files Browse the repository at this point in the history
`gateway_request` in `gateway.manager` raises `tornado.web.HTTPError` exceptions,
but the callers, such as `GatewayKernelManager.get_kernel`, catch
`tornado.httpclient.HTTPError`, instead of `tornado.web.HTTPError`.
Therefore, the callers can not handle exceptions during gateway interactions.

This causes that, for example, when Jupyter Enterprise Gateway culled a kernel
by idle timeout, the gateway manager can not handle the kernel's absent appropriately.
As a result, notebook users see ambiguous "Kernel Error" and can not restart
the kernel or start a new kernel.
  • Loading branch information
shuichiro-makigaki committed Nov 16, 2019
1 parent 9640e1f commit aa73eb4
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
18 changes: 9 additions & 9 deletions notebook/gateway/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,12 +283,12 @@ def gateway_request(endpoint, **kwargs):
except ConnectionRefusedError:
raise web.HTTPError(503, "Connection refused from Gateway server url '{}'. "
"Check to be sure the Gateway instance is running.".format(GatewayClient.instance().url))
except HTTPError:
except HTTPError as e:
# This can occur if the host is valid (e.g., foo.com) but there's nothing there.
raise web.HTTPError(504, "Error attempting to connect to Gateway server url '{}'. "
raise web.HTTPError(e.code, "Error attempting to connect to Gateway server url '{}'. "
"Ensure gateway url is valid and the Gateway instance is running.".
format(GatewayClient.instance().url))
except gaierror as e:
except gaierror:
raise web.HTTPError(404, "The Gateway server specified in the gateway_url '{}' doesn't appear to be valid. "
"Ensure gateway url is valid and the Gateway instance is running.".
format(GatewayClient.instance().url))
Expand Down Expand Up @@ -390,8 +390,8 @@ def get_kernel(self, kernel_id=None, **kwargs):
self.log.debug("Request kernel at: %s" % kernel_url)
try:
response = yield gateway_request(kernel_url, method='GET')
except HTTPError as error:
if error.code == 404:
except web.HTTPError as error:
if error.status_code == 404:
self.log.warn("Kernel not found at: %s" % kernel_url)
self.remove_kernel(kernel_id)
kernel = None
Expand Down Expand Up @@ -559,8 +559,8 @@ def get_kernel_spec(self, kernel_name, **kwargs):
self.log.debug("Request kernel spec at: %s" % kernel_spec_url)
try:
response = yield gateway_request(kernel_spec_url, method='GET')
except HTTPError as error:
if error.code == 404:
except web.HTTPError as error:
if error.status_code == 404:
# Convert not found to KeyError since that's what the Notebook handler expects
# message is not used, but might as well make it useful for troubleshooting
raise KeyError('kernelspec {kernel_name} not found on Gateway server at: {gateway_url}'.
Expand All @@ -587,8 +587,8 @@ def get_kernel_spec_resource(self, kernel_name, path):
self.log.debug("Request kernel spec resource '{}' at: {}".format(path, kernel_spec_resource_url))
try:
response = yield gateway_request(kernel_spec_resource_url, method='GET')
except HTTPError as error:
if error.code == 404:
except web.HTTPError as error:
if error.status_code == 404:
kernel_spec_resource = None
else:
raise
Expand Down
3 changes: 2 additions & 1 deletion notebook/tests/test_gateway.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@

import nose.tools as nt
from tornado import gen
from tornado.httpclient import HTTPRequest, HTTPResponse, HTTPError
from tornado.web import HTTPError
from tornado.httpclient import HTTPRequest, HTTPResponse

from notebook.gateway.managers import GatewayClient
from notebook.utils import maybe_future
Expand Down

0 comments on commit aa73eb4

Please sign in to comment.