consistent handling of any timeout error

some things raise standard TimeoutError, others may raise tornado gen.TimeoutError (gen.with_timeout)

For consistency, add AnyTimeoutError tuple to allow catching any timeout, no matter what kind

Where we were raising `TimeoutError`,
we should have been raising `asyncio.TimeoutError`.

The base TimeoutError is an OSError for ETIMEO, which is for system calls
This commit is contained in:
Min RK
2021-10-19 13:20:48 +02:00
parent 9cf2b5101e
commit 9adbafdfb3
8 changed files with 27 additions and 21 deletions

View File

@@ -90,6 +90,7 @@ from .log import CoroutineLogFormatter, log_request
from .proxy import Proxy, ConfigurableHTTPProxy from .proxy import Proxy, ConfigurableHTTPProxy
from .traitlets import URLPrefix, Command, EntryPointType, Callable from .traitlets import URLPrefix, Command, EntryPointType, Callable
from .utils import ( from .utils import (
AnyTimeoutError,
catch_db_error, catch_db_error,
maybe_future, maybe_future,
url_path_join, url_path_join,
@@ -2350,7 +2351,7 @@ class JupyterHub(Application):
continue continue
try: try:
await Server.from_orm(service.orm.server).wait_up(timeout=1, http=True) await Server.from_orm(service.orm.server).wait_up(timeout=1, http=True)
except TimeoutError: except AnyTimeoutError:
self.log.warning( self.log.warning(
"Cannot connect to %s service %s at %s", "Cannot connect to %s service %s at %s",
service.kind, service.kind,
@@ -2428,7 +2429,7 @@ class JupyterHub(Application):
) )
try: try:
await user._wait_up(spawner) await user._wait_up(spawner)
except TimeoutError: except AnyTimeoutError:
self.log.error( self.log.error(
"%s does not appear to be running at %s, shutting it down.", "%s does not appear to be running at %s, shutting it down.",
spawner._log_name, spawner._log_name,
@@ -2792,7 +2793,7 @@ class JupyterHub(Application):
await gen.with_timeout( await gen.with_timeout(
timedelta(seconds=max(init_spawners_timeout, 1)), init_spawners_future timedelta(seconds=max(init_spawners_timeout, 1)), init_spawners_future
) )
except gen.TimeoutError: except AnyTimeoutError:
self.log.warning( self.log.warning(
"init_spawners did not complete within %i seconds. " "init_spawners did not complete within %i seconds. "
"Allowing to complete in the background.", "Allowing to complete in the background.",
@@ -3055,7 +3056,7 @@ class JupyterHub(Application):
await Server.from_orm(service.orm.server).wait_up( await Server.from_orm(service.orm.server).wait_up(
http=True, timeout=1, ssl_context=ssl_context http=True, timeout=1, ssl_context=ssl_context
) )
except TimeoutError: except AnyTimeoutError:
if service.managed: if service.managed:
status = await service.spawner.poll() status = await service.spawner.poll()
if status is not None: if status is not None:

View File

@@ -47,6 +47,7 @@ from ..metrics import TOTAL_USERS
from ..objects import Server from ..objects import Server
from ..spawner import LocalProcessSpawner from ..spawner import LocalProcessSpawner
from ..user import User from ..user import User
from ..utils import AnyTimeoutError
from ..utils import get_accepted_mimetype from ..utils import get_accepted_mimetype
from ..utils import maybe_future from ..utils import maybe_future
from ..utils import url_path_join from ..utils import url_path_join
@@ -1021,7 +1022,7 @@ class BaseHandler(RequestHandler):
await gen.with_timeout( await gen.with_timeout(
timedelta(seconds=self.slow_spawn_timeout), finish_spawn_future timedelta(seconds=self.slow_spawn_timeout), finish_spawn_future
) )
except gen.TimeoutError: except AnyTimeoutError:
# waiting_for_response indicates server process has started, # waiting_for_response indicates server process has started,
# but is yet to become responsive. # but is yet to become responsive.
if spawner._spawn_pending and not spawner._waiting_for_response: if spawner._spawn_pending and not spawner._waiting_for_response:
@@ -1168,7 +1169,7 @@ class BaseHandler(RequestHandler):
try: try:
await gen.with_timeout(timedelta(seconds=self.slow_stop_timeout), future) await gen.with_timeout(timedelta(seconds=self.slow_stop_timeout), future)
except gen.TimeoutError: except AnyTimeoutError:
# hit timeout, but stop is still pending # hit timeout, but stop is still pending
self.log.warning( self.log.warning(
"User %s:%s server is slow to stop (timeout=%s)", "User %s:%s server is slow to stop (timeout=%s)",

View File

@@ -44,6 +44,7 @@ from . import utils
from .metrics import CHECK_ROUTES_DURATION_SECONDS from .metrics import CHECK_ROUTES_DURATION_SECONDS
from .metrics import PROXY_POLL_DURATION_SECONDS from .metrics import PROXY_POLL_DURATION_SECONDS
from .objects import Server from .objects import Server
from .utils import AnyTimeoutError
from .utils import exponential_backoff from .utils import exponential_backoff
from .utils import url_path_join from .utils import url_path_join
from jupyterhub.traitlets import Command from jupyterhub.traitlets import Command
@@ -718,7 +719,7 @@ class ConfigurableHTTPProxy(Proxy):
_check_process() _check_process()
try: try:
await server.wait_up(1) await server.wait_up(1)
except TimeoutError: except AnyTimeoutError:
continue continue
else: else:
break break

View File

@@ -15,8 +15,6 @@ from subprocess import Popen
from tempfile import mkdtemp from tempfile import mkdtemp
from urllib.parse import urlparse from urllib.parse import urlparse
if os.name == 'nt':
import psutil
from async_generator import aclosing from async_generator import aclosing
from sqlalchemy import inspect from sqlalchemy import inspect
from tornado.ioloop import PeriodicCallback from tornado.ioloop import PeriodicCallback
@@ -38,12 +36,14 @@ from .objects import Server
from .traitlets import ByteSpecification from .traitlets import ByteSpecification
from .traitlets import Callable from .traitlets import Callable
from .traitlets import Command from .traitlets import Command
from .utils import AnyTimeoutError
from .utils import exponential_backoff from .utils import exponential_backoff
from .utils import maybe_future from .utils import maybe_future
from .utils import random_port from .utils import random_port
from .utils import url_path_join from .utils import url_path_join
# FIXME: remove when we drop Python 3.5 support if os.name == 'nt':
import psutil
def _quote_safe(s): def _quote_safe(s):
@@ -1263,7 +1263,7 @@ class Spawner(LoggingConfigurable):
timeout=timeout, timeout=timeout,
) )
return r return r
except TimeoutError: except AnyTimeoutError:
return False return False

View File

@@ -1,16 +1,13 @@
"""Tests for jupyterhub internal_ssl connections""" """Tests for jupyterhub internal_ssl connections"""
import sys import sys
import time import time
from subprocess import check_output
from unittest import mock from unittest import mock
from urllib.parse import urlparse
import pytest import pytest
from requests.exceptions import ConnectionError from requests.exceptions import ConnectionError
from requests.exceptions import SSLError from requests.exceptions import SSLError
from tornado import gen
import jupyterhub from ..utils import AnyTimeoutError
from .test_api import add_user from .test_api import add_user
from .utils import async_requests from .utils import async_requests
@@ -35,7 +32,7 @@ async def wait_for_spawner(spawner, timeout=10):
assert status is None assert status is None
try: try:
await wait() await wait()
except TimeoutError: except AnyTimeoutError:
continue continue
else: else:
break break

View File

@@ -21,6 +21,7 @@ from ..objects import Server
from ..spawner import LocalProcessSpawner from ..spawner import LocalProcessSpawner
from ..spawner import Spawner from ..spawner import Spawner
from ..user import User from ..user import User
from ..utils import AnyTimeoutError
from ..utils import new_token from ..utils import new_token
from ..utils import url_path_join from ..utils import url_path_join
from .mocking import public_url from .mocking import public_url
@@ -95,7 +96,7 @@ async def wait_for_spawner(spawner, timeout=10):
assert status is None assert status is None
try: try:
await wait() await wait()
except TimeoutError: except AnyTimeoutError:
continue continue
else: else:
break break

View File

@@ -26,6 +26,7 @@ from .metrics import RUNNING_SERVERS
from .metrics import TOTAL_USERS from .metrics import TOTAL_USERS
from .objects import Server from .objects import Server
from .spawner import LocalProcessSpawner from .spawner import LocalProcessSpawner
from .utils import AnyTimeoutError
from .utils import make_ssl_context from .utils import make_ssl_context
from .utils import maybe_future from .utils import maybe_future
from .utils import url_path_join from .utils import url_path_join
@@ -707,7 +708,7 @@ class User:
db.commit() db.commit()
except Exception as e: except Exception as e:
if isinstance(e, gen.TimeoutError): if isinstance(e, AnyTimeoutError):
self.log.warning( self.log.warning(
"{user}'s server failed to start in {s} seconds, giving up".format( "{user}'s server failed to start in {s} seconds, giving up".format(
user=self.name, s=spawner.start_timeout user=self.name, s=spawner.start_timeout
@@ -764,7 +765,7 @@ class User:
http=True, timeout=spawner.http_timeout, ssl_context=ssl_context http=True, timeout=spawner.http_timeout, ssl_context=ssl_context
) )
except Exception as e: except Exception as e:
if isinstance(e, TimeoutError): if isinstance(e, AnyTimeoutError):
self.log.warning( self.log.warning(
"{user}'s server never showed up at {url} " "{user}'s server never showed up at {url} "
"after {http_timeout} seconds. Giving up".format( "after {http_timeout} seconds. Giving up".format(

View File

@@ -23,12 +23,12 @@ from operator import itemgetter
from async_generator import aclosing from async_generator import aclosing
from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.exc import SQLAlchemyError
from tornado import gen
from tornado import ioloop from tornado import ioloop
from tornado import web from tornado import web
from tornado.httpclient import AsyncHTTPClient from tornado.httpclient import AsyncHTTPClient
from tornado.httpclient import HTTPError from tornado.httpclient import HTTPError
from tornado.log import app_log from tornado.log import app_log
from tornado.platform.asyncio import to_asyncio_future
# For compatibility with python versions 3.6 or earlier. # For compatibility with python versions 3.6 or earlier.
# asyncio.Task.all_tasks() is fully moved to asyncio.all_tasks() starting with 3.9. Also applies to current_task. # asyncio.Task.all_tasks() is fully moved to asyncio.all_tasks() starting with 3.9. Also applies to current_task.
@@ -97,6 +97,10 @@ def make_ssl_context(keyfile, certfile, cafile=None, verify=True, check_hostname
return ssl_context return ssl_context
# AnyTimeoutError catches TimeoutErrors coming from asyncio, tornado, stdlib
AnyTimeoutError = (gen.TimeoutError, asyncio.TimeoutError, TimeoutError)
async def exponential_backoff( async def exponential_backoff(
pass_func, pass_func,
fail_message, fail_message,
@@ -182,7 +186,7 @@ async def exponential_backoff(
if dt < max_wait: if dt < max_wait:
scale *= scale_factor scale *= scale_factor
await asyncio.sleep(dt) await asyncio.sleep(dt)
raise TimeoutError(fail_message) raise asyncio.TimeoutError(fail_message)
async def wait_for_server(ip, port, timeout=10): async def wait_for_server(ip, port, timeout=10):