skip_before_action causes all controllers to skip the action

[tlhampton13]

Checklist

• [X ] Upgrade Jets: Are you using the latest version of Jets? This allows Jets to fix issues fast. There's a jets upgrade command that makes this a simple task. There's also an Upgrading Guide: http://rubyonjets.com/docs/upgrading/
• [ X] Reproducibility: Are you reporting a bug others will be able to reproduce and not asking a question. If you're unsure or want to ask a question, do so on https://community.rubyonjets.com
• [X ] Code sample: Have you put together a code sample to reproduce the issue and make it available? Code samples help speed up fixes dramatically. If it's an easily reproducible issue, then code samples are not needed. If you're unsure, please include a code sample.

My Environment

Software	Version
Operating System	OSX
Jets	2.3.15
Ruby	2.5.5

---

Expected Behaviour

When a controller uses the skip_before_action to skip a before_action callback declared in a super class, only the controller(s) declaring skip_before_action should skip the callback. All other derived controllers should still call the before_action callback.

Current Behavior

When any controller uses the skip_before_action to skip a before_action callback declared in a super class, all controllers skip the callback.

Step-by-step reproduction instructions

1. add a before_action callback to the ApplicationController class
2. create a controller derived from ApplicationController
3. create a second controller derived from ApplicationController and use the skip_before_action to skip the callback created in ApplicationController

Notice, The first controller should still execute the before_action callback but it doesn't.

Code Sample

In the following code, TestAController should execute the before_callback. TestBController should not execute the before_callback.

class ApplicationController < Jets::Controller::Base
  before_action :before

  def before_callback
    puts "ApplicationController::before called"
  end
end

class TestAController < ApplicationController
  def index
    puts "TestAController::index called"
  end
end

class TestBController < ApplicationController
  skip_before_action :do_before

  def index
    puts "TestBController::index called"
  end
end

Solution Suggestion

[tongueroo]

Am not seeing the same behavior 🤔

app/controllers/application_controller.rb

class ApplicationController < Jets::Controller::Base
  before_action :before_callback
private
  def before_callback
    @before_callback = true
    puts "ApplicationController::before called".color(:yellow)
  end
end

app/controllers/foo1_controller.rb

class Foo1Controller < ApplicationController
  skip_before_action :before_callback
  def index
    render json: {foo: 1, before_callback: !!@before_callback}
  end
end

app/controllers/foo2_controller.rb

class Foo2Controller < ApplicationController
  def index
    render json: {foo: 2, before_callback: !!@before_callback}
  end
end

config/routes.rb

Jets.application.routes.draw do
  root "jets/public#show"
  get "foo1", to: "foo1#index"
  get "foo2", to: "foo2#index"
  any "*catchall", to: "jets/public#show"
end

Debugging:

$ curl localhost:8888/foo1
{"foo":1,"before_callback":false}
$ curl localhost:8888/foo2
{"foo":2,"before_callback":true}
$

Tested this on Jets 2.3.18, though dont see anything the changelog mentioning this issue. Closing out until can reproduce.

[tlhampton13]

Don't know what I was doing wrong before, but all seems to be working fine now for me as well. Thanks for looking at it.

[egold]

FWIW I'm seeing this same behavior, but only in AWS / production. Not locally.

I have recently deployed a new controller that does skip_before_action on a request authenticating method (authenticate_request) and subsequently other controllers are no longer authenticating requests.

[egold]

@tongueroo I am not super familiar with how the Lambdas get generated, I'm trying to grok that now. But it seems like something about the execution environment or generated code makes this happen in Lambda but not when running the server locally.

[egold]

Maybe #304 or #342 might have introduced a bug?

[tlhampton13]

When I re-tested this, I only tried it in my local env. After changing the product and deploying to lambda I saw the failure again and had to take the skip_before_action out again. Works locally, but not when deployed to AWS as @egold described.

[SteffanPerry]

This issue is reproducible in rspec with only option present @egold @tlhampton13 were you using the only param? ie skip_before_action :action, only: %i[meth]

[SteffanPerry]

@tongueroo @egold @tlhampton13,
The PR 559 fixes an issues where the skip_before_action is accompanied by a only param. I left in WIP status as I can not confirm that fixes this exact issue as I could no repro without sending in options to the method. This may be a separate but related bug.

[tongueroo]

A repo to help debug this issue: https://github.com/tongueroo/debug-jets-skip-before-action It's not exactly what's in this report but it shows an issue with the first request. Unsure why.

[SteffanPerry]

awesome thanks looking into this shortly

[SteffanPerry]

Ok after deploying your branch I was able to repro with the failure:

{"foo":1,"action":"a","before_callback":false,"message":"should be false"}
{"foo":1,"action":"a","before_callback":false,"message":"should be false"}
{"foo":1,"action":"b","before_callback":false,"message":"should be false"}
{"foo":1,"action":"b","before_callback":false,"message":"should be false"}
{"foo":2,"action":"a","before_callback":false,"message":"should be true"} # <---
{"foo":2,"action":"a","before_callback":true,"message":"should be true"}
{"foo":2,"action":"b","before_callback":false,"message":"should be true"} # <---
{"foo":2,"action":"b","before_callback":true,"message":"should be true"}

No failures using branch: #559

{"foo":1,"action":"a","before_callback":false,"message":"should be false"}
{"foo":1,"action":"a","before_callback":false,"message":"should be false"}
{"foo":1,"action":"b","before_callback":false,"message":"should be false"}
{"foo":1,"action":"b","before_callback":false,"message":"should be false"}
{"foo":2,"action":"a","before_callback":true,"message":"should be true"}
{"foo":2,"action":"a","before_callback":true,"message":"should be true"}
{"foo":2,"action":"b","before_callback":true,"message":"should be true"}
{"foo":2,"action":"b","before_callback":true,"message":"should be true"}

[tongueroo]

@SteffanPerry Thanks! Merged and released in v3.0.7