You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We (Devolutions) are looking at switching to the new casbin-rs project, leaving our initial port of casbin to Rust from a year ago behind in favour of this one. Overall, the new project looks good, but our primary concern is the lack of proper error handling.
A quick search in the code base reveals there are 92 unwrap() calls. Some of them are benign in nature, like unwrap calls in unit tests, but there are a bunch of them that could cause issues in production. We intend to run this code inside a server process, and unwrap() would cause the entire thread to panic, which is obviously not desirable.
Our old code had a simple error type with an enumeration of a few errors (evaluation error, parsing error, invalid value, etc.). The new code has a simple string error type, but it should be improved with a real enumeration of errors to facilitate error processing.
The idea would be to replace all unwrap() calls with errors, which would affect the function signatures (it would return a Result, which is either a success value or an error value). In addition to this, the error type would be improved with an enumeration of error types. Once functions are modified to return Results with the same error type, it should become easy to use the ? operator to reduce the amount of boilerplate error handling.
We would like to know what would work best to make these kind of improvements. Should we do it ourselves according to the way we have described it and submit a PR, or would you prefer doing it? The side effect of adding proper error handling is that changes will affect a lot of files.
Please let us know, and we'll begin putting our efforts on the new casbin-rs. Thanks!
The text was updated successfully, but these errors were encountered:
@awakecoding Sorry for the late reply. You are right, the current error handling is still very simple, only a CasbinError, would be great if we have a finer error handling (I was too lazy when doing #19). I believe this project hasn't been widely used (maybe only I and GopherJ), It would be awesome if you can help, PRs are welcomed, Thanks.
We (Devolutions) are looking at switching to the new casbin-rs project, leaving our initial port of casbin to Rust from a year ago behind in favour of this one. Overall, the new project looks good, but our primary concern is the lack of proper error handling.
A quick search in the code base reveals there are 92 unwrap() calls. Some of them are benign in nature, like unwrap calls in unit tests, but there are a bunch of them that could cause issues in production. We intend to run this code inside a server process, and unwrap() would cause the entire thread to panic, which is obviously not desirable.
Our old code had a simple error type with an enumeration of a few errors (evaluation error, parsing error, invalid value, etc.). The new code has a simple string error type, but it should be improved with a real enumeration of errors to facilitate error processing.
The idea would be to replace all unwrap() calls with errors, which would affect the function signatures (it would return a Result, which is either a success value or an error value). In addition to this, the error type would be improved with an enumeration of error types. Once functions are modified to return Results with the same error type, it should become easy to use the ? operator to reduce the amount of boilerplate error handling.
We would like to know what would work best to make these kind of improvements. Should we do it ourselves according to the way we have described it and submit a PR, or would you prefer doing it? The side effect of adding proper error handling is that changes will affect a lot of files.
Please let us know, and we'll begin putting our efforts on the new casbin-rs. Thanks!
The text was updated successfully, but these errors were encountered: