Session crashes when setting "connection_timeout" too high

[chStaiger]

linked to chStaiger/iBridges-Gui#266
Maximum for "connection_timeout" in irods_environment.json is 9208512000 which translates to 292 years.
Add extra check and catching of OverflowError: timestamp too large to convert to C _PyTime_t when session is created.

Maybe we can also give the hint in the docs and in the error catching that 1 week corresponds to 604800 seconds.

[qubixes]

Should this be something that is fixed in iBridges though? I suppose we can at least update the readthedocs, but otherwise shouldn't this be handled in the PRC? The only thing we do is change the default timeout, which does not give an error.

[trel]

@d-w-moore do we need to add a too large, but not infinite, number to this test?

https://github.com/irods/python-irodsclient/blob/527417da41a997198e514e0e211b5422585a97a0/irods/test/connection_test.py#L150-L163

[chStaiger]

We should certainly add the info to the FAQ which number translates to which timeframe and that the maximum number is the number of seconds of 292 years :)

[qubixes]

@chStaiger I see that a new branch was created, are you making the adjustments?

[d-w-moore]

@d-w-moore do we need to add a too large, but not infinite, number to this test?

https://github.com/irods/python-irodsclient/blob/527417da41a997198e514e0e211b5422585a97a0/irods/test/connection_test.py#L150-L163

Probably ... what I wonder is - what should be our goal here for the PRC. The error-dump below shows me that the blow-up occurs in the lower-level socket code. Should PRC become a gatekeeper for this case and artificially limit the value. If so, we chould have an 'Infinite' or LARGEST_CONNECTION_TIMEOUT constant as a hard-limit, and/or additionally document it in the README

>>> s=h.make_session(connection_timeout = 9308512000)
>>> s.connection_timeout
9308512000
>>> s.collections.get('/tempZone/home/rods')
Traceback (most recent call last):
  File "/home/daniel/python-irodsclient/irods/session.py", line 322, in server_version
    return tuple(ast.literal_eval(reported_vsn))
  File "/usr/lib/python3.8/ast.py", line 59, in literal_eval
    node_or_string = parse(node_or_string, mode='eval')
  File "/usr/lib/python3.8/ast.py", line 47, in parse
    return compile(source, filename, mode, flags,
  File "<unknown>", line 0
    
    ^
SyntaxError: unexpected EOF while parsing

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/daniel/python-irodsclient/irods/session.py", line 328, in __server_version
    conn = next(iter(self.pool.active))
StopIteration

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/daniel/python-irodsclient/irods/pool.py", line 70, in get_connection
    conn = self.idle.pop()
KeyError: 'pop from an empty set'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/daniel/python-irodsclient/irods/manager/collection_manager.py", line 22, in get
    query = self.sess.query(Collection).filter(*filters)
  File "/home/daniel/python-irodsclient/irods/session.py", line 286, in query
    return Query(self, *args, **kwargs)
  File "/home/daniel/python-irodsclient/irods/query.py", line 45, in __init__
    if self.sess.server_version >= col.min_version:
  File "/home/daniel/python-irodsclient/irods/session.py", line 324, in server_version
    return self.__server_version()
  File "/home/daniel/python-irodsclient/irods/session.py", line 331, in __server_version
    conn = self.pool.get_connection()
  File "/home/daniel/python-irodsclient/irods/pool.py", line 17, in method_
    ret = method(self,*s,**kw)
  File "/home/daniel/python-irodsclient/irods/pool.py", line 87, in get_connection
    conn = Connection(self, self.account)
  File "/home/daniel/python-irodsclient/irods/connection.py", line 62, in __init__
    self._server_version = self._connect()
  File "/home/daniel/python-irodsclient/irods/connection.py", line 245, in _connect
    s = socket.create_connection(address, timeout)
  File "/usr/lib/python3.8/socket.py", line 793, in create_connection
    sock.settimeout(timeout)
OverflowError: timestamp too large to convert to C _PyTime_t
>>> 
>>> 

[d-w-moore]

Testing looks dicey but only because of the Python lib's inconsistency in types of error thrown:

import irods.helpers as h
import sys

largest =   LARGEST_PYTHON_TIMEOUT = 9223372036.854775 # https://stackoverflow.com/questions/45704243/value-of-c-pytime-t-in-python
too_large = LARGEST_PYTHON_TIMEOUT*(1.0+sys.float_info.epsilon)
way_too_large = int(LARGEST_PYTHON_TIMEOUT*(93.0/92))


def f(timeout):
  s=h.make_session(connection_timeout = timeout)
  coll = h.home_collection(s)
  try:
    print('timeout-del = ',timeout, end=' ')
    s.collections.get(coll)
  except Exception:
    print ('** EXCEPTION')
    raise
  else:
    print('ok')

for t in [largest,too_large, way_too_large]:
   try:
      print ('============================================================')
      f(t)
   except Exception as e:
      print('excinfo',type(e))

gives:

$ python conntimeout.py 
============================================================
timeout-del =  9223372036.854774 ok
============================================================
timeout-del =  9223372036.854776 ** EXCEPTION
excinfo <class 'ValueError'>
============================================================
timeout-del =  9323626080 ** EXCEPTION
excinfo <class 'OverflowError'>

[trel]

Can’t we just catch both in PRC? Is that weird / confusing?

[d-w-moore]

Can’t we just catch both in PRC? Is that weird / confusing?

We can do that. It was just confusing that the type of Error depends on the amount by which we exceeded the threshold

[trel]

Agreed. But, seems well behaved enough for us to react consistently and protect our PRC users (hi!).

[trel]

note comments at the top of 3.12...

https://github.com/python/cpython/blob/3.12/Include/cpython/pytime.h

are gone in 3.13...

https://github.com/python/cpython/blob/3.13/Include/cpython/pytime.h

with more words now at

https://github.com/python/cpython/blob/3.13/Doc/c-api/time.rst

[trel]

also, just for the record...

clamping our PRC session timeout to 292 years seems... sufficient :)

[chStaiger]

@chStaiger I see that a new branch was created, are you making the adjustments?

Yes, I will update the doc and will insert some error catching.

[qubixes]

@chStaiger I see that a new branch was created, are you making the adjustments?

Yes, I will update the doc and will insert some error catching.

I propose that we don't do any error catching for now. After we update the docs, almost no one should run into the issue.

[chStaiger]

Yes, I just threw a warning and corrected the number. PR will follow asap.

[qubixes]

Yes, I just threw a warning and corrected the number. PR will follow asap.

Even that I wouldn't do, especially if it going to get handled in the PRC or future versions of CPython. We could make another item in the FAQ?