Audit ErrorKind names for reasonableness and adherence to convention.
See discussion at !325 (comment 2779199) .
We should go through all the existing ErrorKinds and the errors we map to them and make sure that we're correctly distinguishing "Local" vs "Remote" vs "Tor" as their prefixes.
Here's the breakdown of what I think we should do.
I'm putting the “
ErrorKinds to change
These errors, I think, might become BadApiUsage
:
-
AlreadyClosed
(1 use) — using an object that you've already closed. -
NamespaceFull
(1 use) — putting too many streams on a circuit, or too many circuits on a channel. A well-behaved client should never actually do this. -
PersistentStateReadOnly
(1 use) — writing to a read-only state is totally a thing you're not supposed to do.
These errors become TorNetworkError
:
-
CircuitCollapse
(1 use) — your stream failed because the Tor circuit went away. -
RemoteIdMismatch
(1 use) — you found a relay other than the one expected. -
RemoteRefused
(2 uses) — one use will be eliminated by !329 (merged). The other use is for cases where a relay replies with aDESTROY
to yourEXTEND
request.
These errors should not be treated as errors, but rather as normal return conditions:
-
RemoteNameError
(2 uses) — This is the exit telling you about an NXDOMAIN or suchlike. -
RequestedResourceAbsent
(1 use) — This is an error-case from the unused relay-side of our ntor implementation.
I think these errors should get renamed, or lumped into other errors:
-
Network
(9 uses) — Rename toLocalNetworkError
for consistency -
UnexplainedTaskSpawnFailure
(1 use) — Perhaps this should just beInternal
.❓ -
NoHomeDirectory
(1 use) — This seems awfully specific. Maybe we should change this to something more general, meaning "there is a problem with your local environment."❓
I think these errors needs a better name:
-
TorConnectionFailed
(3 uses) — To me, this name doesn't say "I was unable to connect to the Tor network and bootstrap." Also, it doesn't follow the convention that names starting withTor
refer to a problem on the Tor network: this one is much more likely to refer to a problem with the local network.❓ -
TorShuttingDown
(1 use) — Violates the convention that names starting withTor
refer to a problem on the Tor network. Maybe rename toArtiShuttingDown
or lump together withReactorShuttingDown
?❓
These errors seem dubious to me, and probably want a certain amount of redesign. I don't propose to handle that as a part of this ticket, though:
-
Canceled
(2 uses) — used for two very different purposes. One use-case will go away with #347 (closed); the other use-case means that our configuration changed out from under us, or all circuits were deliberately retired, before a circuit could complete. -
TransientFailure
(1 use) — used when a circuit is perfectly good but we decide not to use it.
ErrorKinds to leave unchanged
kind | count |
---|---|
BadApiUsage |
10 |
BootstrapRequired |
5 |
CacheAccessFailed |
3 |
CacheCorrupted |
6 |
DirectoryExpired |
2 |
FeatureDisabled |
0 |
ForbiddenStreamTarget |
1 |
Internal |
32 |
InvalidConfig |
6 |
InvalidConfigTransition |
1 |
InvalidStreamTarget |
1 |
LocalProtocolViolation |
2 |
NoExit |
7 |
NoPath |
8 |
NotImplemented |
8 |
PersistentStateAccessFailed |
1 |
PersistentStateCorrupted |
1 |
ReactorShuttingDown |
1 |
RemoteNetworkTimeout |
2 |
RemoteStreamClosed |
1 |
RemoteStreamError |
1 |
TorNetworkError |
1 |
TorNetworkTimeout |
3 |
TorProtocolViolation |
19 |