-
Notifications
You must be signed in to change notification settings - Fork 10
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
Feature/simplify error handling #1264
base: develop
Are you sure you want to change the base?
Conversation
Test Results 25 files - 42 25 suites - 42 34s ⏱️ -45s For more details on these failures, see this check. Results for commit c0287f9. ± Comparison against base commit 1bd270a. This pull request removes 874 and adds 5 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
22d88ec
to
9da8284
Compare
logger.info("Returning server web exchange error ${this.type} (${this.message})") | ||
exchange.response.statusCode = this.status | ||
exchange.response.headers[CONTENT_TYPE] = MediaType.APPLICATION_JSON_VALUE | ||
val body = serializeObject(this) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here, you are also serializing the inherited Exception
class, which is not the expected thing.
btw, we should also get rid of this inherited class (in FP, errors should not be exceptions). not sure of the impacts, to be checked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here, you are also serializing the inherited Exception class, which is not the expected thing.
Yes fixed
btw, we should also get rid of this inherited class (in FP, errors should not be exceptions). not sure of the impacts, to be checked.
ApiException are thrown 14 times. And each time the refacto could impact multiple layer of functions.
shared/src/main/kotlin/com/egm/stellio/shared/model/ApiExceptions.kt
Outdated
Show resolved
Hide resolved
|
||
import java.net.URI | ||
|
||
sealed class ErrorResponse( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be the ultimate hierarchy for representing errors (https://old.arrow-kt.io/docs/patterns/error_handling/)
384b5c1
to
16509a4
Compare
Detail is now used to redirect people to troubleshoot.
a42108c
to
33c90bb
Compare
Quality Gate passedIssues Measures |
No description provided.