Error with rack-mount and rails 4.2.4 #1151

brauliobo · 2015-09-12T00:28:46Z

The following error happens only with rails 4.2.4 (and rack 1.6) and not on rails 3.2.22 (and rack 1.4):

ERROR["test_should_suggest_event_children", ArticlesTest, 2015-09-10 18:40:02 -0300]
 test_should_suggest_event_children#ArticlesTest (1441921202.61s)
NoMethodError:         NoMethodError: undefined method `[]' for nil:NilClass
            /home/braulio/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-mount-0.8.3/lib/rack/mount/route_set.rb:155:in `block in call'
            /home/braulio/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-mount-0.8.3/lib/rack/mount/code_generation.rb:96:in `block in recognize'
            /home/braulio/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-mount-0.8.3/lib/rack/mount/code_generation.rb:68:in `optimized_each'
            /home/braulio/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-mount-0.8.3/lib/rack/mount/code_generation.rb:95:in `recognize'
            /home/braulio/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-mount-0.8.3/lib/rack/mount/route_set.rb:141:in `call'
            /home/braulio/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/grape-0.13.0/lib/grape/api.rb:114:in `call'
            /home/braulio/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/grape-0.13.0/lib/grape/api.rb:44:in `call!'
            /home/braulio/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/grape-0.13.0/lib/grape/api.rb:39:in `call'
            /home/braulio/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-test-0.6.3/lib/rack/mock_session.rb:30:in `request'
            /home/braulio/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-test-0.6.3/lib/rack/test.rb:244:in `process_request'
            /home/braulio/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-test-0.6.3/lib/rack/test.rb:67:in `post'
            test/unit/api/articles_test.rb:366:in `block in <class:ArticlesTest>'

The error happened because rack-mount is expecting the result of rack call to be an array, but it is a hash (file rack-mount-0.8.3/lib/rack/mount/route_set.rb):

151             
152         result = route.app.call(env) 
153             
154         pp result
155         if result[1][X_CASCADE] == PASS
156           env[@parameters_key] = old_params
157         else
158           return res

It seems that rack-mount is unmaintained for a long time (even the repo was deleted). Have you considered to not depend on it anymore?

cc @diguliu @terceiro

The text was updated successfully, but these errors were encountered:

dblock · 2015-09-12T13:25:42Z

We've discussed rack-mount in #1072, nothing really came out of it, would love to replace it.

Grape did change the response to be a Rack::Response, and this is not supposed to happen, do you have a vanilla project in which we can see this?

brauliobo · 2015-09-12T13:34:35Z

@dblock yeah, just checkout https://github.com/noosfero/noosfero.git, branch rails4, then run

BACKTRACE=blegga ruby -Itest test/unit/api/articles_test.rb -n /suggest_event_children/

We were using grape 0.13.0, is the change to Rack::Response there?

dblock · 2015-09-12T13:40:24Z

This was in 0.12.0, https://github.com/ruby-grape/grape/blob/master/UPGRADING.md#upgrading-to--0120, every time an issue like this has come up it seemed like the problem was in rack-mount but it wasn't.

brauliobo · 2015-09-12T13:46:26Z

@dblock the strange thing is that this is single failing test, among many others. I don't see any grape's middleware used.

brauliobo · 2015-09-12T14:36:40Z

@dblock an exception is happening! and as it happens it crashes the rack-mount! Do you know why?

dblock · 2015-09-13T13:32:48Z

Lets reopen this, I'll take a look when I have a few.

brauliobo · 2015-09-13T13:34:16Z

@dblock it didn't happen on rails 3.2, so maybe it is a new way rails4.2/rack1.6 is issuing exceptions

dblock · 2015-09-14T12:37:18Z

This test passes on my machine, I have updated Grape to 0.13. How do I reproduce the problem?

~/source/noosfero/noosfero (rails4)$ BACKTRACE=blegga ruby -Itest test/unit/api/articles_test.rb -n /suggest_event_children/
Started with run options -n /suggest_event_children/ --seed 37437

  1/1: [==================================================================================================================] 100% Time: 00:00:01, Time: 00:00:01

Finished in 1.74757s
1 tests, 2 assertions, 0 failures, 0 errors, 0 skips
~/source/noosfero/noosfero (rails4)$ bundle show rails
/Users/dblock/.rvm/gems/ruby-2.1.2/gems/rails-4.2.4
~/source/noosfero/noosfero (rails4)$ bundle show grape
/Users/dblock/.rvm/gems/ruby-2.1.2/gems/grape-0.13.0

brauliobo · 2015-09-14T12:39:48Z

@dblock we fixed the exception that was happening on the AR model. Maybe we can make a test to reproduce this inside grape.

brauliobo · 2015-09-14T12:41:01Z

@dblock maybe a simple raise 'here' somewhere in the model or maybe even in the api classes is enough

dblock · 2015-09-14T12:41:11Z

Do you have a commit or a way to put back the exception?

dblock · 2015-09-14T12:57:32Z

The problem is this:

      rescue_from :all do |e|
        logger.error e
      end

If you rescue an error you're not supposed to swallow it. It's clearly not well documented, but what's happening here is that logger.error ... just returns true, so the result from the middleware is [true, nil, nil], which isn't what Rack expects.

Help me edit the README to explain this?

A simple fix is

rescue_from :all

This causes the following response:

[500, {"Content-Type"=>"application/json", ...]

If you want to log, you want something like this:

rescue_from :all do |e|
   logger....
   error! e.message, 500
end

You definitely want to fix this in your codebase or unexpected errors will never be returned to the API clients. Write a test that causes an exception and makes sure the error is valid JSON for example.

brauliobo · 2015-09-14T13:08:44Z

thanks a lot @dblock, I'll test this.

on noosfero/noosfero@190f639 the test should still FAIL

dblock · 2015-09-14T13:09:29Z

See my explanation above. LMK if you need help.

…ll error./script/quick-start Closes #1151. [ci-skip]

see ruby-grape/grape#1151 (comment)

brauliobo · 2015-09-14T13:16:57Z

@dblock nice, just applied your suggestion on error handling: noosfero/noosfero@8bc107b

Looking to the README all its examples show this. We really missed that, gonna contact the api developers.

ka8725 · 2015-10-03T01:55:09Z

Seems the problem is still present when using rack/test in tests + warden:

     # /usr/local/opt/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-mount-0.8.3/lib/rack/mount/route_set.rb:154:in `block in call'
     # /usr/local/opt/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-mount-0.8.3/lib/rack/mount/code_generation.rb:96:in `block in recognize'
     # /usr/local/opt/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-mount-0.8.3/lib/rack/mount/code_generation.rb:68:in `optimized_each'
     # /usr/local/opt/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-mount-0.8.3/lib/rack/mount/code_generation.rb:95:in `recognize'
     # /usr/local/opt/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-mount-0.8.3/lib/rack/mount/route_set.rb:141:in `call'
     # /usr/local/opt/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/grape-0.13.0/lib/grape/api.rb:114:in `call'
     # /usr/local/opt/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/grape-0.13.0/lib/grape/api.rb:44:in `call!'
     # /usr/local/opt/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/grape-0.13.0/lib/grape/api.rb:39:in `call'
     # /usr/local/opt/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-test-0.6.3/lib/rack/mock_session.rb:30:in `request'
     # /usr/local/opt/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-test-0.6.3/lib/rack/test.rb:244:in `process_request'
     # /usr/local/opt/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/rack-test-0.6.3/lib/rack/test.rb:67:in `post'

dblock · 2015-10-03T19:09:49Z

@ka8725 If you can have a very simple repro that would be helpful.

ka8725 · 2015-10-03T21:39:57Z

@dblock take this repo. Update grape to 0.13.0. Run it with rackup and curl localhost:9292/foo.

Note, the code works with the old version. But, actually, I don't think that the problem in Grape. Possible fix in warden.

dblock · 2015-10-04T15:11:15Z

This makes sense. I opened #1174. I'd like some note in UPGRADING about this error and known issues with middleware such as Warden. Please feel free to PR anything that would be useful for the next person.

dblock added the bug? label Sep 12, 2015

brauliobo closed this as completed Sep 12, 2015

dblock reopened this Sep 13, 2015

dblock closed this as completed Sep 14, 2015

dblock added a commit that referenced this issue Sep 14, 2015

Clarify that rescue_from must return a Rack::Response, re-raise or ca…

d3841f1

…ll error./script/quick-start Closes #1151. [ci-skip]

brauliobo added a commit to noosfero/noosfero that referenced this issue Sep 14, 2015

rails4: correctly handle api errors

8bc107b

see ruby-grape/grape#1151 (comment)

ka8725 mentioned this issue Oct 3, 2015

Ready for Rack::Response wardencommunity/warden#119

Merged

dblock mentioned this issue Oct 4, 2015

NoMethodError: undefined method `[]' for nil:NilClass w/ Warden #1174

Closed

frobichaud mentioned this issue Oct 14, 2015

Warden dependency causes issue with grape mcordell/grape_token_auth#17

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Error with rack-mount and rails 4.2.4 #1151

Error with rack-mount and rails 4.2.4 #1151

brauliobo commented Sep 12, 2015

dblock commented Sep 12, 2015

brauliobo commented Sep 12, 2015

dblock commented Sep 12, 2015

brauliobo commented Sep 12, 2015

brauliobo commented Sep 12, 2015

dblock commented Sep 13, 2015

brauliobo commented Sep 13, 2015

dblock commented Sep 14, 2015

brauliobo commented Sep 14, 2015

brauliobo commented Sep 14, 2015

dblock commented Sep 14, 2015

dblock commented Sep 14, 2015

brauliobo commented Sep 14, 2015

dblock commented Sep 14, 2015

brauliobo commented Sep 14, 2015

ka8725 commented Oct 3, 2015

dblock commented Oct 3, 2015

ka8725 commented Oct 3, 2015

dblock commented Oct 4, 2015

Error with rack-mount and rails 4.2.4 #1151

Error with rack-mount and rails 4.2.4 #1151

Comments

brauliobo commented Sep 12, 2015

dblock commented Sep 12, 2015

brauliobo commented Sep 12, 2015

dblock commented Sep 12, 2015

brauliobo commented Sep 12, 2015

brauliobo commented Sep 12, 2015

dblock commented Sep 13, 2015

brauliobo commented Sep 13, 2015

dblock commented Sep 14, 2015

brauliobo commented Sep 14, 2015

brauliobo commented Sep 14, 2015

dblock commented Sep 14, 2015

dblock commented Sep 14, 2015

brauliobo commented Sep 14, 2015

dblock commented Sep 14, 2015

brauliobo commented Sep 14, 2015

ka8725 commented Oct 3, 2015

dblock commented Oct 3, 2015

ka8725 commented Oct 3, 2015

dblock commented Oct 4, 2015