Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Custom error pages #32

Closed
jodosha opened this issue Jun 30, 2014 · 7 comments · Fixed by #65
Closed

Custom error pages #32

jodosha opened this issue Jun 30, 2014 · 7 comments · Fixed by #65
Labels
enhancement help-wanted

Comments

@jodosha
Copy link
Member

jodosha commented Jun 30, 2014

Allow developers to customize their error pages.

As now, when a request that accepts text/html and causes a non successful response, the framework shows a simple page like the following:

<!DOCTYPE html>
<html>
  <head>
    <title>Internal Server Error</title>
  </head>
  <body>
    <h1>Internal Server Error</h1>
  </body>
</html>

This use case is handled by Lotus::Views::Default which renders lotus/templates/default.html.erb.
@twe4ked
Copy link

twe4ked commented Jul 1, 2014

I'll take a crack at this. Might be a few days as I'm in and out of internet.

@simi
Copy link
Contributor

simi commented Jul 8, 2014

We can inspire here - https://github.com/hassox/rack-rescue/blob/master/lib/rack/rescue.rb for production and I think we can add https://github.com/charliesome/better_errors into default generated app Gemfile (and make it optionable in generator). I don't see any reason why we should duplicate better_errors code.

@jodosha
Copy link
Member Author

jodosha commented Jul 8, 2014

@simi The scope of this ticket is to allow to customize production error pages.

While better_errors is for development purposes. Because Lotus depends on Rack, we already have Rack::ShowExceptions as an alternative.

@simi
Copy link
Contributor

simi commented Jul 8, 2014

So plan is to add custom configurable rack middleware to production middleware stack to handle errors?

@jodosha
Copy link
Member Author

jodosha commented Jul 8, 2014

@simi Where did you read that? Rack::ShowExceptions is for development as well, and it's out of scope here.

Right now we have the scenario depicted in the description of this ticket.

The idea is to let developers to add a file like apps/bookshelf/templates/500.html.erb, where they can define the production error page.

@mengqing mengqing mentioned this issue Jul 31, 2014
Closed
@jodosha
Copy link
Member Author

jodosha commented Jul 31, 2014

Reporting here an idea of for implementation, that I had in a discussion with @twe4ked .

If you look at the signature of a view's constructor it's: #initialize(template, locals). Lotus::View.render is also a factory: it instantiate a view, passing the right template.

What if we override it?

def self.render(root, template_name, context)
  if template = DefaultTemplateFinder.new(root, template_name).find
    new(template, context).render # the developer has defined a custom template
  else
    super(context) # render the default view
  end
end

..and

class Lotus::Views::DefaultTemplateFinder < Lotus::View::Rendering::TemplateFinder
  def initialize(root, template_name)
     @root = root
     @options = { template_name: template_name }
  end

  private
  def root
    @root
  end
end

.. to be used like:

# lib/lotus/rendering_policy.rb
Lotus::Views::Default.render(@templates, response.code, {response: response, format: :html})

@twe4ked
Copy link

twe4ked commented Jul 31, 2014

I got this working but never had time to finish the tests. I'll try get to it soon but if anyone else gets to it first that's fine. Sorry it's taken so long. I assume we would want integration tests for this? I created a PR (#61) of what I've got so far.

@pkurek pkurek mentioned this issue Aug 5, 2014
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement help-wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

added support for custom error pages
3 participants
@jodosha @twe4ked @simi