Removed Result<> from auth handlers#14
Conversation
evgenymng
left a comment
evgenymng
left a comment
There was a problem hiding this comment.
I didn't see any glaring issues with these changes, although personally I am not a fan of the boilerplate code that was added (explicit error handling), and I would much prefer a more elegant (and uniform) solution, like custom errors.
| Ok(hash) => hash, | ||
| Err(_) => { | ||
| return HttpResponse::InternalServerError().body("Decoding password hash error") | ||
| } | ||
| }; |
There was a problem hiding this comment.
Is the formatting fine? Please, confirm by running cargo fmt.
There was a problem hiding this comment.
@ProstoLive Ты можешь просто rebase сделать теперь, чтобы проверить форматирование.
@evgenymng Давай будем на русском общаться в PR-ах, пока не будет необходимости делать иначе.
| user.password = match bcrypt::hash(user.password, bcrypt::DEFAULT_COST) { | ||
| Ok(hashed_password) => hashed_password, | ||
| Err(_) => { | ||
| return HttpResponse::BadRequest().body("Hashing password error"); |
There was a problem hiding this comment.
В функции login ошибка bcrypt::hash интерпретируется как InternalServerError, так что надо выбрать что-то одно. Считаю, что это должно быть ISE, потому что фильтрацией ввода должен заниматься не бэкенд.
| return HttpResponse::BadRequest().body("Hashing password error"); | |
| return HttpResponse::InternalServerError().body("Hashing password error"); |
|
Предлагаю закрыть этот PR, так как Issue создавалась до смещения фокуса разработки. P.S: К тому же у @t3m8ch на подходе PR с обработкой ошибок. |
4 participants
No description provided.