Skip to content

Conversation

@sschroe
Copy link
Contributor

@sschroe sschroe commented May 3, 2017

This adds some visible output on 404 and 500 errors using internal pre-defined html.

These internal strings are fine as a fallback but later on there should be themed and customizable versions of these. But for this we first need to think of a template engine and the overall directory structure (where should those error pages be located?).

I used a function for this to avoid stuffing the strings into the HTTP handling. Albeit this probably isn't the prettiest solution yet.

This is for issue #51

Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for contributing. What do you think of my proposals?

match status {
status::NotFound =>
(ContentType::html().0, status, "
<html>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay since we want to have it hard coded what about the usage of include_str!?


if !path.exists() {
return Ok(Response::with(status::InternalServerError));
return Ok(Response::with(get_http_error_as_html(status::NotFound)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid the function(in_function(in_function(...))) calls we ca add a new Trait to iron::status::Status which returns our HTML in case on an error.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.03%) to 32.751% when pulling 83c50a3 on sschroe:master into 8f23abf on rust-leipzig:master.

@sschroe
Copy link
Contributor Author

sschroe commented May 4, 2017

include_str! seems fine but we probably should have a subdirectory for these static resources to avoid cluttering the src folder with it.

For the Trait option i'm somewhat indifferent, i suppose it could be nicer if we don't have to add a whole bunch of other stuff around it for that to work.

@saschagrunert
Copy link
Member

Alright then I propose to use the include_str!(...) there.

@saschagrunert saschagrunert merged commit cff49c9 into rust-leipzig:master May 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants