Maybe flatten out exception tree
Nobody has claimed this yet.
Assessment
- Difficulty
- 5/5
- Estimated time
- Over a week
- Newbie friendliness
- 25/100
Research direction
Start by reviewing the current exception hierarchy and the proposed exceptions.general, exceptions.http, exceptions.opening, exceptions.kv_store, and exceptions.acl modules. Then inspect the generator and its collision handling. Done means agreeing on the hierarchy and names, implementing the flattening without unintended parentage changes, and adding collision safeguards.
Written by the indexing model from the issue text.
Description
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, andHttpIncompletetoexceptions.http, though. - It's unfortunate
GenericErrorexists, as it sound a lot likeUnexpectedFastlyError.UnexpectedFastlyErrorshould become private, as it indicates a bug in our code if it's thrown. People definitely shouldn't be catching it. CatchExceptionorFastlyExceptionif you want a catch-all. - Rename
GenericErrortoOtherErrorto 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:ErrorWithDetailwith a better name, likeHttpError.- All the
send-error-detailexceptionserror-with-detailcarries (not currently generated). Map anyErrorWithDetailthat has (optional) details directly onto these, subclassingHttpError. - All the
trailer-errors. RenameErrortoOtherError.
-
exceptions.opening:- All
open-errorerrors. This is actually a pretty semantically cohesive category.
- All
-
exceptions.kv_store:- All
kv-errorerrors. - There's another
GenericErrorthere. Rename it toOtherError.
- All
-
exceptions.acl:- All the
acl-errors. RenameGenericErrortoOtherError.
- All the
Open questions
- Are we going to cause trouble for ourselves by changing the parentage of some errors, like making
error'sHttpInvalidsuperclassexceptions.http.HttpError(or perhaps some other common ancestor)? I rather think so. The common-ancestor approach sounds good. Theerrorsuperclass is worthless, but theerror-with-detailone 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.
- Dominant language
- Python
- Stars
- 5
- Forks
- 1
- PR merge metrics
- No merged PRs in 30d
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from fastly/compute-sdk-python
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
fastly/compute-sdk-python#116 ·
-
Difficulty 4/5 3-5 days Newbie friendliness 35/100
fastly/compute-sdk-python#98 ·
-
Difficulty 5/5 Over a week Newbie friendliness 35/100
fastly/compute-sdk-python#74 ·
-
Difficulty 5/5 Over a week Newbie friendliness 35/100
fastly/compute-sdk-python#61 ·
-
fastly/compute-sdk-python#60 · 1 assignee ·
All issues in fastly/compute-sdk-python
Similar issues
-
essnmx good first issue
Difficulty 1/5 Under an hour Newbie friendliness 95/100
-
[Feature] 奇物选择添加优先级 Open
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
syfoud/Simulated_Scepter#174 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
Giskard-AI/giskard-oss#2840 · 1 comment ·
-
A claim comment carrying the issue number is silently declined while the workflow reports success Openarea: repo bug perceived difficulty: 2
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
yeti-platform/yeti#1380 ·