Maybe flatten out exception tree

[erikrose]

This is a ticket for discussion and design, responding to Paul's comment about nested exceptions being awkward to reach for.

--

I spread all the errors out on the table and had a look. We can get a pretty nice result by dividing exceptions across 5 modules, while still saving ourselves from utterly changing the shape of the world (fighting the WIT forevermore). No more subpackages under exceptions, just 5 files:

• exceptions.general:

  • All from types.error. I don't see a point in having a common superclass for these at all; there's no semantic, and it doesn't really help catchers narrow down what threw it, as many things throw these.
  • Move HttpInvalid, HttpUser, and HttpIncomplete to exceptions.http, though.
  • It's unfortunate GenericError exists, as it sound a lot like UnexpectedFastlyError. UnexpectedFastlyError should become private, as it indicates a bug in our code if it's thrown. People definitely shouldn't be catching it. Catch Exception or FastlyException if you want a catch-all.
  • Rename GenericError to OtherError to get a little closer to its meaning. "Generic" suggests to me a superset of the other errors, while "Other" unambiguously indicates it's not one of those errors.

• exceptions.http:

  • ErrorWithDetail with a better name, like HttpError.
  • All the send-error-detail exceptions error-with-detail carries (not currently generated). Map any ErrorWithDetail that has (optional) details directly onto these, subclassing HttpError.
  • All the trailer-errors. Rename Error to OtherError.

• exceptions.opening:

  • All open-error errors. This is actually a pretty semantically cohesive category.

• exceptions.kv_store:

  • All kv-error errors.
  • There's another GenericError there. Rename it to OtherError.

• exceptions.acl:

  • All the acl-errors. Rename GenericError to OtherError.

Open questions

• Are we going to cause trouble for ourselves by changing the parentage of some errors, like making error's HttpInvalid superclass exceptions.http.HttpError (or perhaps some other common ancestor)? I rather think so. The common-ancestor approach sounds good. The error superclass is worthless, but the error-with-detail one may have meaning.

Name collisions (if we were to flatten everything into 1 module):

• Error (x2), but we can probably remove the types.error.Error common superclass to make that go away.
• GenericError (x4)
• LimitExceeded (x2)
• TooManyRequests (x2)
• Unsupported (x2)

Paths not taken

We could deduplicate and have, say, ACL stuff and non-ACL stuff throw the same TooManyRequests error, but we'd be losing potentially important info for the catcher. So that's a bad idea. (Also, it could get messy if the 2 TooManyRequests errors later diverge in shape, which might happen if WIT grows subtyping.)

Thus, we must rename exceptions or divide up into modules.

If we flatten and rename, we get into trouble in the future when somebody introduces another InternalError and we never prefixed the KV one into KvStoreInternalError. I'm against preemptively prefixing the heck out of everything, because it gets wordy and this isn't Java. And does one really need to say HttpTrailerError? There are unlikely to be other kinds of trailer errors. But if we don't prefix universally, it becomes awkward to program against because you constantly have to look aside to see what we named things. So let's keep the names short and just make a better __repr__ instead if needed to clarify tracebacks.

Prerequisites

We'd need to add alarm bells about collisions in the generator so we don't accidentally template duplicate names into one module.

[posborne]

I'm personally not against prefixing things and find it helps avoid namespace conflicts in code in the way that imports used in practice (from x.y.z import Error). I would prefer to have the mapping be consistent with fewer special cases, even if that means that there are a few cases where we end up with something slightly less desirable (but a more direct mapping to the WIT).