-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Custom Error Types #35
Comments
Hi Kevin, apologies for the delay in replying to this, I had missed the notification and only spotted this when I saw the other issue you raised. Custom error types were not a goal of this library when it was written, and would likely require some significant changes, which would be complicated by trying to keep backwards compatibility with existing usage. It's likely not impossible, but not something we would have time to look into right now. I'll leave this open as a reminder for it being something to look into though, and thanks for the suggestion! |
Yea, totally makes sense. As an alternative I've considered converting all the int32 error codes into strings to be more flexible with external systems. As a side note, is there any reason the current ErrorInfo string is stored within the Error object as a shared pointer? Is that simply an optimization? Thanks for the followups! |
I believe so, though it's unclear how useful that actually is, as I suspect |
Good to know. Thanks! |
Curious, how hard would it be to allow for this library to support custom error types? One of the biggest advantages of using a type for error handling vs. traditionally supporting exception based promise handling is that you can actually parse and typecheck your errors.
As it stands, the custom error type is fairly opinionated: requiring an error code, error context code (which must be integers), and only being flexible on the error info as a string.
What would be the most flexible way to support custom error types beyond a string? I took a stab at looking at how hard it would be templatize the error, but it seems like this would actually be a massive change across the board.
The text was updated successfully, but these errors were encountered: