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

local scope variables are not available inside rescue_from #1922

Open
aruprakshit opened this issue Oct 23, 2019 · 6 comments
Open

local scope variables are not available inside rescue_from #1922

aruprakshit opened this issue Oct 23, 2019 · 6 comments

Comments

@aruprakshit
Copy link

aruprakshit commented Oct 23, 2019

I have current_user defined inside the before hook, and then we try to read it from rescue_from method, we get nil.

before do
  token         = nil
  token         = headers.fetch("Authorization", "").split("Basic ")[1] unless report_url?
  @current_user = ::User.find_by(auth_token: token) if token.present?

  PaperTrail.request.whodunnit = @current_user.id if @current_user
end

helpers Pundit

helpers do
  attr_reader :current_user

  def report_url?
    request.path_info == "/v1/share_reports/#{request.params['token']}"
  end

  # ....
end


rescue_from Pundit::NotAuthorizedError do |e|
  Airbrake.notify(e, {
    user: {
      id: current_user.id,
      email: current_user.email
    }
  })

  error!(
    {
      messages: {
        userNotification: e.message
      }
    }, 401
  )
end

We get nil error when current_user being accessed inside the rescue_from block as current_user.id. How can I make available current_user inside the rescue_from block?

Gemfile

gem "rails", "5.2.3"
gem 'grape', '~> 1.0'
gem 'grape-entity', '~> 0.7.1'
@dblock dblock added the bug? label Oct 23, 2019
@dblock
Copy link
Member

dblock commented Oct 23, 2019

I think I'd want this to work. Doing @something = in before makes it available in the execution of the API (we made that work a long time ago), but not in rescue_from. Let's call it a bug or a non-feature. Maybe someone could try to fix it?

As a workaround, the rescue block is not executed in the same scope as the request, so you don't have access to attributes. You do have access to context (since #1918, in a released version you can reach into ENV), which gives you the endpoint and then gives you endpoint-based context and settings. I think you generally want to do what Warden does, which is using ENV to pass around something across various contexts of the request (before, during, after) and not @.

@aruprakshit
Copy link
Author

@dblock For now I worked it around in my code by following this merged code. But once, it will be released I think I can use context.

current_user is available when I do access it like below inside the helpers block.

def signed_in_user
  # check https://github.com/ruby-grape/grape/pull/1918/files
  # We will remove it, once the grape 1.5 is released.
  env[Grape::Env::API_ENDPOINT].current_user
end

@dblock
Copy link
Member

dblock commented Oct 23, 2019

Yep. I will leave this open to make @variable work. At the very least we should document what the scope of @ is in a rescue_from block.

@aruprakshit This is a good place for someone to write some tests and contribute :) Wink wink

@dblock dblock changed the title helpers are not available inside the rescue_from block - Why? local scope variables are not available inside rescue_from Oct 23, 2019
@jcagarcia
Copy link
Contributor

jcagarcia commented Nov 21, 2023

Hey 👋

I was taking a look at this issue and after some checks I can see that we are in a different instance when we are inside the rescue_from method.

require 'grape'

class MyAPI < Grape::API
  class MyError < StandardError; end

  before do
    puts "self => #{self.class}"
    @my_var = 328
  end

  rescue_from MyError do |e|
    puts "self => #{self.class}"
    puts "my var value => #{@my_var}"
    error!({ my_var: @my_var }, 401)
  end

  get '/wadus' do
    puts "my var value => #{@my_var}"
    puts "self => #{self.class}"
    raise MyError.new
  end
end

When entering on the before method, the puts method shows: #<Class:0x0000000107c79d88>
When entering on the endpoint, the puts method shows: #<Class:0x0000000107c79d88> (same as before)
However, when we are inside the rescue_from, the puts method shows: #<Class:0x0000000104c77950>

This means that we cannot have access to the same instance variables declared in the before because we are not in the same instance.

I have prepared a PR with some failing tests expecting that when rescuing, the error contains the same value that we set in a instance variable in the before validation.

For the solution I have a little idea.

My feeling is that when performing run_rescue_handler instead of performing instance_exec(&handler), we should do it like endpoint.instance_exec(&handler) where the endpoint is the instance obtained from @env["api.endpoint"] here. Doing it in that way, we are executing the handler under the endpoint instance scope and we have the variables accessible.

self => #<Class:0x000000010448a148>
my var value => 328
self => #<Class:0x000000010448a148>
self => #<Class:0x000000010448a148>
my var value => 328

However, I'm getting this error because as I'm executing the handler in the endpoint instance, the error! method that I'm using is defined in the inside_route.rb:164, that is throwing again the error.

UncaughtThrowError: uncaught throw :error
	/Users/Juancarlos.Garcia/code/grape/lib/grape/dsl/inside_route.rb:167:in `throw'
	/Users/Juancarlos.Garcia/code/grape/lib/grape/dsl/inside_route.rb:167:in `error!'
	/Users/Juancarlos.Garcia/code/grape-2236/api.rb:14:in `block in <class:MyAPI>'
	/Users/Juancarlos.Garcia/code/grape/lib/grape/middleware/error.rb:135:in `instance_exec'

When using the error.rb instance for performing the handler, the error! method is defined in the errors.rb:53 and is just returning the response and not throwing a new error.

Also, some other methods are failing in the tests following this approach, as the user can use methods like rack_response inside a rescue_from that are not defined in the endpoint instance.

As an alternative (and as a test) I just copied the instance variables from the endpoint instance into the new instance like

endpoint.instance_variables.each do |variable|
  instance_variable_set(variable, endpoint.instance_variable_get(variable))
end

With this, and performing the instance_exec(&handler) under the errors instance, everything works fine and the @ variable can be used inside the rescue_from.

self => #<Class:0x0000000105839d88>
my var value => 328
self => #<Class:0x0000000105839d88>
self => #<Class:0x00000001028379f0>
my var value => 328

I think that the first approach is the ideal, because we are performing the handler in the same context of the endpoint. However, same method error! in the endpoint instance needs to do something different if used inside the rescue_from context. Also, there are some other methods like rack_response that can be called inside the rescue_from context but NOT inside the endpoint context.

What do you think about the second approach @dblock ? Can you give me some light here? :) Happy to hear from someone else as well :)

@dblock
Copy link
Member

dblock commented Nov 22, 2023

Thanks for the detailed explanation! I don't love copying instance variables at all because modifying them will not work across copies, so I think fixing self is the way to go. I understand the other concerns.

@jcagarcia
Copy link
Contributor

Totally agree @dblock . I've just tried to copy the instance variables from one instance to another to see if after that they were accessible.

I've applied the context fix solution in #2377

jcagarcia added a commit to jcagarcia/grape that referenced this issue Nov 23, 2023
…the instance variables behavior. Extra tests added
jcagarcia added a commit to jcagarcia/grape that referenced this issue Nov 23, 2023
jcagarcia added a commit to jcagarcia/grape that referenced this issue Nov 24, 2023
dblock pushed a commit that referenced this issue Nov 24, 2023
…inside rescue_from (#2377)

* fix(#1922): Allow to use instance variables defined in the endpoints when rescue_from

* fix(#1922): Update CHANGELOG and running rubocop

* fix(#1922): Updating UPGRADING and README files explaining the instance variables behavior. Extra tests added

* fix(#1922): Send endpoint parameter as the last param of the run_rescue_handler method

* fix(#1922): Adding short before/after example to UPGRADING

* fix(#1922): Fixing CHANGELOG format style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants