Opened 16 years ago
Closed 16 years ago
#145 closed enhancement (wontfix)
make DeadReferenceError qualify as RemoteException
| Reported by: | Brian Warner | Owned by: | |
|---|---|---|---|
| Priority: | major | Milestone: | 0.5.0 |
| Component: | error-handling | Version: | 0.4.1 |
| Keywords: | Cc: |
Description
Tahoe#889 indicates a place where some logging code was surprised to see DeadReferenceError qualify as a "local" exception (and thus got logged with more severity than intended). When I wrote that logging code, I had expected DeadReferenceError to qualify as remote, via f.check(RemoteException).
I'd like to fix this, so that code which does f.check(RemoteException) will catch DeadReferenceError. On the other hand, it is useful for clients to spot DeadReferenceError easily, because they typically respond to it by dropping the connection or trying to establish a new one (as opposed to other remote errors that don't affect the connection).
I'd like to make this use of f.check work for both the DeadReferenceErrors that occur when you lose the connection while the request is outstanding, and for the ones that occur when the connection is already gone by the time you call callRemote. These are delivered via two separate code paths. I believe (but haven't confirmed) that the former code path wraps the DeadReferenceError in a RemoteException, but the latter does not.
So I'm considering making DeadReferenceError simply inherit from RemoteException. The downside of this would be that code which does f.check(RemoteException) could not also just do f.value.failure.check, because these DeadReferenceErrors would not have a .failure attribute. This would be a significant compatibility burden.
The alternative would be to have the callRemote-side path also wrap its DeadReferenceError in a RemoteException. This would means that application code would never see callRemote errback with DeadReferenceError, so code which wanted to handle lost connections specially would have to be updated. This affects backwards compatibility too.
Change History (2)
comment:1 Changed 16 years ago by
| Component: | unknown → error-handling |
|---|
comment:2 Changed 16 years ago by
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |

Looking at the code paths, it appears that
DeadReferenceErroris never wrapped inRemoteException. DRE is created in three places:callRemote/Broker.newRequestId(sending on an already-broken rref)PendingRequest.failduringBroker.abandonAllRequests(connection loss, mapped fromConnectionLostorConnectionDone)Broker.abandonAllRequestsfromNegotiationwhen a connection is replaced by a newer oneAnd
RemoteExceptionis only created on receipt of an(error)sequence, which is only sent byErrorSlicer.So the current behavior is at least consistent.
I guess I'll leave this alone, because the compatibility problems are hard. Existing code that uses the current API will have trouble with moving
DeadReferenceErrorunderRemoteException(code which log.WEIRDs anything but RemoteException will create noisy incidents for mere lost connections, and code which reacts to DeadReferenceError would stop working).Instead, application code should anticipate the following categories of exceptions (when
setOption("expose-remote-exception-types", False)is in effect):RemoteException, on which it is always safe to examinef.value.failure. These can only result from exceptions that occur on the far end of the wire, from either an inbound schema Violation or an exception raised by the remote_* method.DeadReferenceError, caused when the connection is lost before the response is received (possibly before the request was even sent). A serious banana-level error will drop the connection, causingDeadReferenceErrorfor all outstanding requests.Violation, caused when a local inbound schema violation is detected, because the remote end sent us something unexpectedSo application code that should probably have a handler that looks like this:
def handle(f): if f.check(RemoteException): if f.value.failure.check(IndexError): return "remote indexerror" if f.value.failure.check(AppSpecificError): return "known" return f if f.check(DeadReferenceError): self.stop_using_this_connection() return None return f # something local