apple / ccs-calendarserver

The Calendar and Contacts Server.
https://www.calendarserver.org
Apache License 2.0
485 stars 136 forks source link

If the connection factory is mis-defined, reconnection logic retries forever and never fails #389

Open macosforgebot opened 10 years ago

macosforgebot commented 10 years ago

exarkun@… originally submitted this as ticket:824


Consider an ill defined connection factory such as:

    connectionFactory = lambda label: Connection("localhost")

This factory is defined wrong because it requires the label argument rather than making it optional (as the documentation directs it must be).

However, the failure mode for this circumstance is unfortunate. If this is used to construct a ConnectionPool then the result is a pool for which no operations ever succeed or fail. The connection retry logic just keeps trying to call the connectionFactory over and over again forever.

The exception is logged, which is quite nice (since it means identifying the problem is mostly a matter of remembering to read the log file). The logged traceback looks like this:

2013-11-27 08:34:19-0500 [-] Re-trying connection due to connection failure
        Traceback (most recent call last):
          File "/home/exarkun/Projects/Twisted/branches/releases/release-13.1.0-6575/twisted/python/context.py", line 81, in callWithContext
            return func(*args,**kw)
          File "twext/internet/threadutils.py", line 48, in _run
            while self._qpull():
          File "twext/internet/threadutils.py", line 69, in _qpull
            return True
          File "twext/internet/threadutils.py", line 82, in _oneWorkUnit
            self._reactor.callFromThread(deferred.callback, result)
        --- <exception caught here> ---
          File "twext/internet/threadutils.py", line 74, in _oneWorkUnit
            result = instruction()
          File "twext/enterprise/adbapi2.py", line 1025, in initCursor
            connection = self.connectionFactory()
        exceptions.TypeError: <lambda>() takes exactly 1 argument (0 given)

Of course, differentiating this failure from other more ... legitimate ... failures (eg failures that are not due to bugs in the connection factory but are due to a real, perhaps transient, problem establishing a conection to the SQL server - such as networking issues, the SQL server not being running, etc) may be tricky.

Perhaps it would be feasible to catch TypeError and treat it as a fatal error, though. Or perhaps a TypeError with only one frame of stack trace would be safer. Or a TypeError coupled with an inspect.getargspec check to make a better determination that the source of the problem is really an incorrect signature.

These ideas likely each has its own drawback. Perhaps someone else can think of a better approach, though.

macosforgebot commented 10 years ago

@wsanchez originally submitted this as comment:1:⁠ticket:824

macosforgebot commented 10 years ago

@wsanchez originally submitted this as comment:2:⁠ticket:824