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.

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

default_error being reset #35

Open
matisojka opened this issue Oct 5, 2013 · 0 comments
Open

default_error being reset #35

matisojka opened this issue Oct 5, 2013 · 0 comments

Comments

@matisojka
Copy link

I have been trying to use a default 404 handling in my app, so I look in the source code / documentation and found that it is possible to pass a valid Rack-style object that responds to #call:

class HttpRouter
  # (...) omitted on purpose


  # Creates a new HttpRouter.
  # Can be called with either <tt>HttpRouter.new(proc{|env| ... }, { .. options .. })</tt> or with the first argument omitted.
  # If there is a proc first, then it's used as the default app in the case of a non-match.
  # Supported options are
  # * :default_app -- Default application used if there is a non-match on #call. Defaults to 404 generator.
  # * :ignore_trailing_slash -- Ignore a trailing / when attempting to match. Defaults to +true+.
  # * :redirect_trailing_slash -- On trailing /, redirect to the same path without the /. Defaults to +false+.
  def initialize(*args, &blk)
    default_app, options     = args.first.is_a?(Hash) ? [nil, args.first] : [args.first, args[1]]
    @options                 = options
    @default_app             = default_app || options && options[:default_app] || proc{|env| ::Rack::Response.new("Not Found", 404, {'X-Cascade' => 'pass'}).finish }
    @ignore_trailing_slash   = options && options.key?(:ignore_trailing_slash) ? options[:ignore_trailing_slash] : true
    @redirect_trailing_slash = options && options.key?(:redirect_trailing_slash) ? options[:redirect_trailing_slash] : false
    @route_class             = Route
    reset!
    instance_eval(&blk) if blk
  end

So I tried instantiating the router like this:

    @route_resolver = HttpRouter.new(proc { |env|
      error_404_handler.call(env)
    })

In this case, error_404_handler should be responding with a custom message but it wasn't.

So looking further in the source code, I discovered a rather strange thing: when instantiating the HttpRouter class, the first argument can be a callable object, and this object is going to be the default_app, called when there is no matching route. But as we can see in the #initialize method, after defining the default_app, the method #reset! is getting called, which basically resets some stuff and the default_app.

I fixed it putting the #reset! call right at the beginning of #initialize:

def initialize(*args, &blk)
    reset!
    default_app, options     = args.first.is_a?(Hash) ? [nil, args.first] : [args.first, args[1]]
    @options                 = options
    @default_app             = default_app || options && options[:default_app] || proc{|env| ::Rack::Response.new("Not Found", 404, {'X-Cascade' => 'pass'}).finish }
    @ignore_trailing_slash   = options && options.key?(:ignore_trailing_slash) ? options[:ignore_trailing_slash] : true
    @redirect_trailing_slash = options && options.key?(:redirect_trailing_slash) ? options[:redirect_trailing_slash] : false
    @route_class             = Route
    instance_eval(&blk) if blk
  end

As I don't know enough about the code to know if this is going to break other stuff, I'd like to get some feedback on it first, before proposing a PR. Thanks!

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

No branches or pull requests

1 participant