2

We have a use case for mounting a mock engine to process sessions when developing locally in which a custom session middleware calls the mock engine via Net::http request when a request comes through.

When there is code change, the reloader is triggered and here calls the ActiveSupport::Dependencies to start unloading. Then the request is pass down to our custom session middleware and the http request is fired. However since the http request calls to a mountable engine, it goes thought the same middlewares again and the reloader unloads all the dependencies again which cause the first reload to timeout. So the goal is to be able to skip the reload for the second request.

I added the follow code to ActionDispatch::Reloader here and it does exactly what I wanted. 

class Reloader < Executor
  def initialize(app, executor)
    super(app, executor)
  end

  def call(env)
    request = ActionDispatch::Request.new(env)
    return @app.call(env) if skip_request?(request)
    super(env)
  end

  def skip_request?(request)        
    request.path_info.start_with?('/session')
  end
end

Then I want to make this cleaner figured to pull that out completely to a module and just do a swap like this from an initializer 

app.config.middleware.swap(::ActionDispatch::Reloader, MyModule::CustomReloaderMiddleware)

Here is the module

require 'action_dispatch'

module MyModule
  class CustomReloaderMiddleware < ActionDispatch::Executor
    def initialize(app, executor)
      @app, @executor = app, executor
    end

    def call(env)
      request = ActionDispatch::Request.new(env)
      return @app.call(env) if skip_request?(request)
      super(env)
    end

    def skip_request?(request)
      request.path_info.start_with?('/session')
    end
  end
end

But I ran into a couple issues.

Uncaught exception: wrong number of arguments (given 1, expected 2) from for the initialize in MyModule, when I starts the server. Then I tried the following

#1

def initialize(app, executor = nil)
  @app, @executor = app, executor
end

#2

def initialize(app, executor = nil)
  @app, @executor = app, ActiveSupport::Reloader
end

Both of them starts the service correctly and I see the request going through this middleware but it does not reload the code.. So I wondered what is the correct way of swapping ActionDispatch::Reloader with a custom reloader ?

AirWick219
  • 894
  • 8
  • 27
  • Sorry, I've had limited blocks of free time lately, and never came back to dig into your issue. This sounds like a good solution. – matthewd Aug 15 '18 at 06:21
  • O no worry !! you have been a great help and I have learned so much from what you wrote !! – AirWick219 Aug 15 '18 at 06:45

1 Answers1

2

You need to pass your middleware's additional argument to the swap call:

app.config.middleware.swap(::ActionDispatch::Reloader, MyModule::CustomReloaderMiddleware, app.reloader)

That's the same argument given when ActionDispatch::Reloader is first added -- it's the application's reloader, which is a more specifically configured subclass of AS::Reloader (so you were on the right track).

matthewd
  • 4,332
  • 15
  • 21
  • That works !! Thank you so much !! Through this process I learned so much about the reloader. I guess it's a unique use case that we have calling a mock engine from a middleware or do you think the reloader should prevent the second reload ? – AirWick219 Aug 15 '18 at 06:31
  • I still _think_ the second reload should be unnecessary -- because the first one should've gotten everything up to date -- in which case the second should be prevented just by not being tried... if it legitimately thinks it needs to reload again, though, I'm not sure what it should do: skipping means the inner request runs on known-outdated code, which could be very confusing. (Here I'm sure it's fine, because the chances that your dummy session handler have changed seem vanishingly small.) – matthewd Aug 15 '18 at 06:34
  • The mock engine is a gem that we add on to the app for development so it wouldn't ever have a code change on the fly. I think the fact that we are calling the mock engine in the middleware is the issue. From looking at the debugger, it seems like the executor call `run!` on the reloader and pass the request down the next middleware before the reloading is complete. – AirWick219 Aug 15 '18 at 06:41
  • That request to the mock engine from the middleware hit the middlewares from the top again as if a new requests comes in which then unload all the classes and never make it out the middleware and cause the first request to timeout – AirWick219 Aug 15 '18 at 06:44
  • When `run!` returns, the reload is done. (`complete!` is talking about the request that _triggered_ the reload, not the reload itself. Confusing naming ) – matthewd Aug 15 '18 at 06:46
  • `complete!` is the one that calls `self.class.reloaded!` which marks `@should_reload = false` But that is call after `@app.call(env)` which pass the request down the middleware. Although the reloading part is done and `release_unload_lock!` is called, but `@should_reload` was not mark as false until `complete!` – AirWick219 Aug 15 '18 at 06:57
  • Oh! That makes sense, thanks. Seems tricky to solve for the general case (there are some configurations where the actual reloading happens in `complete!`), but that does sound like something worth fixing. – matthewd Aug 15 '18 at 07:47
  • O .. i am not aware that there is config to make the reloading happens in 'complete! '. Do u know what is it call ? I guess it would be some kind of call back. Not sure what that use case is but i am not sure thats a good idea with our case, cuz reloading in 'complete! ' means that the middleware request will hit the old code. But anyhow I am more than happy to submit a pr with your guidance !! – AirWick219 Aug 15 '18 at 08:23
  • But what is the reason to call '@app.call(env)' which pass the request down the middlewares after reloading is done but before 'complete!' is called ?? Then whats the reason wrapping the reloader with a executor at all ? – AirWick219 Aug 15 '18 at 08:30