refactor: move status codes into classes #4200
Conversation
Make each error class have to define its own status code. This makes it easier for developers to see which code an error corresponds to and means less jumping back and forth between files. Unfortunately, the long switch needs to stay in case we get errors thrown that aren't of the Unleash Error type, but we can move it to the `fromLegacyError` file instead.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
There was a problem hiding this comment.
My tradeoff analysis:
+ I like the locality of error to code reasoning
- now HTTP leaks to the non-HTTP code that throws those errors e.g. application services
If we had other delivery mechanisms other than HTTP then it wouldn't make sense to couple error codes to one protocol (HTTP). But since we're mostly doing web it may not be a problem.
Thanks! I like that a lot.
This is a good point and something I hadn't considered. The same data was always available on those errors (by using the same property), I've just made the declaration local to each error instead of something that the parent class handles. The idea was to make it easier to create new error classes with their corresponding error codes. Because the errors are intended to be API errors (or at least, I've always considered them to be that), I think that makes sense. Taking your comment into consideration, I still think it's the right thing to do, but I'm not bullish about it. We could always walk it back later if we find that it's not appropriate. The old code is still available and we could easily enough roll back this change if we find that we want to decouple it later. What do you think? |
|
@thomasheartman let's observe the feedback from the code over time |
Make each error class have to define its own status code. This makes
it easier for developers to see which code an error corresponds to and
means less jumping back and forth between files. In other words: improved locality.
Unfortunately, the long switch needs to stay in case we get errors
thrown that aren't of the Unleash Error type, but we can move it to
the
fromLegacyErrorfile instead.