Decouple from HTTP::Server::Context
#49
Conversation
| @@ -47,7 +47,7 @@ class MyErrorHandler | |||
| raise SomeError.new("Something went wrong") | |||
| rescue e | |||
| context.response.status_code = 500 | |||
| context.response.print MyApp::ExceptionPage.for_runtime_exception(context, e).to_s | |||
| context.response.print MyApp::ExceptionPage.new context, e | |||
There was a problem hiding this comment.
No need to call to_s on this since response.print will do it automatically?
There was a problem hiding this comment.
Correct, with the added benefit of not having to load the entire string into memory since I'm pretty sure it'll be written directly to the response IO.
|
I like the concept. Seems straight forward. Let's ping @crimson-knight and @sdogruyol too since Kemal and Amber also use this and see if they have any thoughts. |
|
It definitely is a breaking change. Two of the constructor overload's signatures changed, an ivar was removed, and the expected type of some constructor args changed. So anyone using those overloads or ivar in custom template wouldn't work. |
|
Yeah, Lucky for sure would need to be re-worked, but thankfully I don't think it would really be a "user" facing breaking change in terms of devs building (Lucky) apps. |
|
Not sure who has review/merging rights in this repo, but bump. |
it has a breaking change crystal-loot/exception_page#49
This PR does a bit of a refactor to decouple an exception page from a
HTTP::Server::Context, making it more flexible. E.g. for use in frameworks where that object is not directly available, but all the required information is.Opening this is a draft to allow time for discussion before moving forward:
HTTP::*types vs hashesHTTP::Statusas an ivar@queryivar in favor of@params@querywas simply@params.to_s.for_runtime_exceptionin favor of.newResolves #48