Revamp error handler/action #903
Conversation
paulcsmith
force-pushed
the
pcs/868-redo-handler
branch
from
0a86782
to
a11e4fd
Compare
|
|
||
| private class FakeErrorAction < Lucky::ErrorAction | ||
| def handle_error(error : FakeError) : Lucky::Response | ||
| def render(error : CustomError) : Lucky::Response |
There was a problem hiding this comment.
Renamed handle_error to render. I think this is simpler and reads better
There was a problem hiding this comment.
Does this cause any issues with calling the render method from actions? I guess it shouldn't if their argument types are different.
There was a problem hiding this comment.
I think it will work (or it wouldn't compile) but this could be kind of confusing or lead to unexpected compile errors since there could be tons of different overloads. I think for now I'll rename it render_error
I've been thinking of renaming render to html or something. Since everything else (like json) has the type in the name. E.g. html Users::IndexPage, users: UserQuery.new
There was a problem hiding this comment.
I like the idea of using html. It's an easy change.
There was a problem hiding this comment.
Done! I like it. And should be a pretty easy thing to upgrade
| plain_text "This is not a debug page", status: 500 | ||
| end | ||
|
|
||
| def report(error : Exception) |
There was a problem hiding this comment.
Added an abstract report method for reporting methods however you want!
| private def call_error_action(context : HTTP::Server::Context, error : Exception) : HTTP::Server::Context | ||
| status_code = status_code_by_error(error) | ||
| context.response.status_code = status_code | ||
| settings.logger.error(exception: error.inspect_with_backtrace) |
There was a problem hiding this comment.
Always log the exception
| it "calls the report method on the error action" do | ||
| io = IO::Memory.new | ||
| FakeErrorAction.temp_config(output: io) do | ||
| handle_error(error: CustomError.new) do |_context, _output| |
There was a problem hiding this comment.
oh, nevermind. I see this method from below.
|
|
||
| private class FakeErrorAction < Lucky::ErrorAction | ||
| def handle_error(error : FakeError) : Lucky::Response | ||
| def render(error : CustomError) : Lucky::Response |
There was a problem hiding this comment.
Does this cause any issues with calling the render method from actions? I guess it shouldn't if their argument types are different.
|
|
||
| private class FakeErrorAction < Lucky::ErrorAction | ||
| def handle_error(error : FakeError) : Lucky::Response | ||
| def render(error : CustomError) : Lucky::Response |
There was a problem hiding this comment.
I think it will work (or it wouldn't compile) but this could be kind of confusing or lead to unexpected compile errors since there could be tons of different overloads. I think for now I'll rename it render_error
I've been thinking of renaming render to html or something. Since everything else (like json) has the type in the name. E.g. html Users::IndexPage, users: UserQuery.new
paulcsmith
force-pushed
the
pcs/868-redo-handler
branch
from
a11e4fd
to
2b7d9aa
Compare
paulcsmith
force-pushed
the
pcs/868-redo-handler
branch
from
2b7d9aa
to
3600ecb
Compare
Part of #868. See that issue for details