Grape: headers set before error! is missing on response

Created on 17 Dec 2017 · 11Comments · Source: ruby-grape/grape

Whenever I execute error!(), response does not contains my headers. In fact none of the headers like X-Request-Id. I presume this is due to calling a new Request instance and return that instead.

How do I make sure that my important headers are in every request that grape gave? Even in Rails::API I am able to hold headers on any error.

Of course, I could add my own Middleware. Is there any better or easy solution before middleware option?

bug?

Source

gencer

👍3

Most helpful comment

I think i found a reason for that case.
That's all because of the Grape::Middleware::Error. The problem is here

          error_response(catch(:error) do
            return @app.call(@env)
          end)

on this stage there are no custom headers set. So the headers set in before callbacks are not reachable.
I found a quick work around:

module Middlewares
  class SomeMiddleware < Grape::Middleware::Base
    def before
      header('A', 1.to_s)
    end
  end
end

then
insert_before Grape::Middleware::Error, Middlewares::SomeMiddleware
But the whole concept of such error catching is wrong IMHO and breaks the concept of before callbacks.

fr33z3 on 11 Jan 2018

👍4

All 11 comments

What are "your" headers? I think anything you set with header ... should be there, but it looks like we don't document this or another behavior - would appreciate a PR with specs and docs.

You can set additional headers on error with

error! 'Unauthorized', 401, 'X-Error-Detail' => 'Invalid token.'

Lets clarify this for everyone.

dblock on 17 Dec 2017

I have few headers that sent to user on every request in every action. I do not want to add them manually on each and every error! call. I already defined them on:

before do
  header ...
  header ...
end

based on request type, user and others. These headers can be different on each request by user or type. So, I would expect grape to respect my before.

Problem is, before gets called but all headers set in before are gone. If this is an expected behavior then I will stick with middlewares.

gencer on 18 Dec 2017

👍3

Does adding a header inside the API work?

get do
   header ...
   errror!
end

If it does, I think before and after should work the same way and it would be a bug.

Care to write some specs?

dblock on 19 Dec 2017

👍1

Note: If you want I can create an another branch with test in spec/ folder.

Expected: Both requests should pass.
Actual: Second test will fail.

error_header_spec.rb:

require 'spec_helper'

describe Grape::API do 
 let(:error_header) do
    Class.new(Grape::API) do
      before do
            header 'X-Grape-Before-Header', 'yeah'
      end
      after do
            header 'X-Grape-After-Header', 'yeah'
      end
      get '/success' do
            header 'X-Grape-Returns-Error', 'yeah'
      end
      get '/error' do
            header 'X-Grape-Returns-Error', 'yeah'
            error!({ success: false })
      end
    end
  end

  subject do
    ErrorHeader = error_header
    Class.new(Grape::API) do
      format :json
      mount ErrorHeader => '/'
    end
  end

  def app
    subject
  end

  it 'should returns all headers on success' do
  get '/success'
    expect(last_response.headers['X-Grape-Returns-Error']).to eq('yeah')
    expect(last_response.headers['X-Grape-Before-Header']).to eq('yeah')
    expect(last_response.headers['X-Grape-After-Header']).to eq('yeah')
  end

  it 'should returns all headers on error' do
  get '/error'
    expect(last_response.headers['X-Grape-Before-Header']).to eq('yeah')
    expect(last_response.headers['X-Grape-After-Header']).to eq('yeah')
    expect(last_response.headers['X-Grape-Returns-Error']).to eq('yeah')
  end
end

gencer on 20 Dec 2017

Just pull request the failing spec please. Reduce the code a bit, there's a bunch of unnecessary stuff like versioning. Extend the spec to all the scenarios for before, after and during the API so we can cover it all and make sure we fix it properly.

dblock on 20 Dec 2017

@dblock, How is it look like now? (As I am new to this rspec world :)) I would like to get confirm before proper pull request

gencer on 20 Dec 2017

👍1

I think i found a reason for that case.
That's all because of the Grape::Middleware::Error. The problem is here

          error_response(catch(:error) do
            return @app.call(@env)
          end)

on this stage there are no custom headers set. So the headers set in before callbacks are not reachable.
I found a quick work around:

module Middlewares
  class SomeMiddleware < Grape::Middleware::Base
    def before
      header('A', 1.to_s)
    end
  end
end

then
insert_before Grape::Middleware::Error, Middlewares::SomeMiddleware
But the whole concept of such error catching is wrong IMHO and breaks the concept of before callbacks.

fr33z3 on 11 Jan 2018

👍4

The above PR does not totally address the issue, IMHO. Errors cannot only be raised by calling error! explicitly. Errors may occur in the block given to the HTTP-verb method like

get do
  Model.find(param[:id]) # may raise an ActiveRecord::NotFoundError
end

or by Grape itsself during a POST request (i.e. before entering my code as defined in the endpoint's post-block) when the entity restricts certain attributes to have a state different from the one in which the attribute was posted.
I expect headers in the before block to be set on the response just as well as if I had called error! myself. This should be addressed in another PR. WDYT?