Use exception_page shard

[dwightwatson]

Description of the Change

Hi there - this is an attempted re-implementation of #864 but using the exception_page shard that was created out of it. Let me first confess that I'm very new to Crystal and Amber so this likely isn't perfect, but it is working.

What I'm not positive for - and would appreciate some guidance on - is how to ensure this won't be displayed in a production environment. I haven't quite worked out how to get Amber running in production mode on my local machine so haven't been able to test it.

Alternate Designs

I customised the page with the Amber logo, project URL and brand colours. It's just a nice feel versus the stock standard layout.

Benefits

Very pretty exception pages which brings us into line with other Crystal frameworks like Kemal and Lucky.

Possible Drawbacks

It does introduce another dependency, but if exception_page is fairly standard across frameworks then it shouldn't be too much of a concern.

[faustinoaq]

@dwightwatson Very nice! I haven't had enough time to work in this, Thank you so much! 👍

What I'm not positive for - and would appreciate some guidance on - is how to ensure this won't be displayed in a production environment.

Check #274, I managed to get Exception pages working on 3 conditions:

1. Amber::Error::Pipe is disabled and shows basic plain text error message.
2. Amber::Error::Pipe is enabled and shows this development template (default)
3. Amber::Error::Pipe is enabled and error_controller.cr is modified to get customized error pages.

Maybe we can use Amber.env.development? to ensure this page works only on development, WDYT?

[dwightwatson]

Thanks for the additional context - good to get a better understanding of how Amber all fits together.

I've had a crack at breaking out a html_response method that will only return the new error page in development, otherwise return the existing plain HTML response. Is this the sort of implementation you were looking for?

[drujensen]

@dwightwatson @faustinoaq should this be included in the amber base controller or should this shard be injected in the end users project via the generators so they can handle how it works?

[dwightwatson]

That didn't occur to me - it should probably be a development dependency too. A quick look into that suggests that only development dependencies in the root shard.yml file are installed - is that correct? In that case I suppose it would have to be injected in the end user's project.

[dwightwatson]

Do you have any additional feedback on this one in order to get it merged in?

[drujensen]

Wait, we have amber g error already. I may be confused as to what this is for. @faustinoaq can you provide some guidance here? I think I am missing something.

https://github.com/amberframework/amber/tree/master/src/amber/cli/templates

[faustinoaq]

@drujensen This error page is suposed to work only on development

I think we should look what others framework did here:

Kemal

https://github.com/kemalcr/kemal/blob/ced34b7651930d513419c9b8188dad8648454676/src/kemal/helpers/exception_page.cr#L4

Lucky

https://github.com/luckyframework/lucky/blob/500e4bc28786722700672c398b768b3937b204cd/src/lucky/exception_page.cr#L1

amber g error is for generate custom error pages like this:

https://amberframework.org/nothing

[drujensen]

@faustinoaq 👍

@dwightwatson Based on @faustinoaq response, I was wrong to ask to have you move the shard to the template. Can you move it back? :sorry:

[drujensen]

@dwightwatson thanks for this contribution. I will look into the shard location and see which is best.

[dwightwatson]

Apologies for not getting back to this sooner, was travelling for work. Thanks for sorting it out 😄