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

Logger middleware just crashes #317

Closed
PavelPenkov opened this issue Oct 28, 2013 · 9 comments
Closed

Logger middleware just crashes #317

PavelPenkov opened this issue Oct 28, 2013 · 9 comments

Comments

@PavelPenkov
Copy link

This simple program crashes because when logger is called there no headers and body yet.

require 'faraday'

conn = Faraday.new(url: "http://google.com") {|config| config.response :logger}
conn.get "/"
/Users/synapse/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/faraday-0.8.8/lib/faraday/response/logger.rb:31:in `dump_headers': undefined method `map' for nil:NilClass (NoMethodError)
    from /Users/synapse/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/faraday-0.8.8/lib/faraday/response/logger.rb:25:in `block in on_complete'
    from /Users/synapse/.rbenv/versions/2.0.0-p247/lib/ruby/2.0.0/logger.rb:376:in `add'
    from /Users/synapse/.rbenv/versions/2.0.0-p247/lib/ruby/2.0.0/logger.rb:404:in `debug'
    from /Users/synapse/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/faraday-0.8.8/lib/faraday/response/logger.rb:25:in `on_complete'
    from /Users/synapse/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/faraday-0.8.8/lib/faraday/response.rb:9:in `block in call'
    from /Users/synapse/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/faraday-0.8.8/lib/faraday/response.rb:63:in `on_complete'
    from /Users/synapse/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/faraday-0.8.8/lib/faraday/response.rb:8:in `call'
    from /Users/synapse/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/faraday-0.8.8/lib/faraday/response/logger.rb:20:in `call'
    from /Users/synapse/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/faraday-0.8.8/lib/faraday/connection.rb:253:in `run_request'
    from /Users/synapse/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/gems/faraday-0.8.8/lib/faraday/connection.rb:106:in `get'
    from faraday_bug.rb:4:in `<main>'
@mislav
Copy link
Contributor

mislav commented Oct 28, 2013

This is expected. None of the middleware can work without HTTP requests being made, i.e. without an adapter.

Faraday.new(url: "http://google.com") do |conn|
  conn.response :logger
  conn.adapter Faraday.default_adapter
end

@mislav mislav closed this as completed Oct 28, 2013
@sethvargo
Copy link

@mislav while this is the expected behavior, the resulting error message could be more useful.

@shepmaster
Copy link

I agree with @sethvargo, the message could be better.

In my case, I went from no parameters to Faraday.new and then added conn.response :logger. I would have expected the adapter to be the default_adapter by default, so I was reasonably confused at the new error. It didn't help that I was debugging another error, so I thought that this might have been another symptom of my original error.

@bluefuton
Copy link

I also ran into this problem. I'd say that the expected behaviour is that Faraday uses the default adapter if one isn't specified.

If you'd rather not do this, an 'adaptor not specified' exception would save a lot of head scratching.

@gerrywastaken
Copy link

@mislav I was about to say exactly what @bluefuton just said. I mean it is called the "default_adapter", which suggests that it's the default one.

@henrik
Copy link

henrik commented Jul 9, 2015

Oh, I was just bitten by the same thing. Cryptic error message for what is likely a common mistake. (I removed the adapter specification because I mistakenly assumed it needn't be explicit.)

@mislav Would you accept a pull request to change the error to something like "No adapter specified"?

Or (but you seem to not want this) to assume the "default adapter" if nothing is explicitly specified?

@henrik
Copy link

henrik commented Jul 9, 2015

Oh, hey @pengwynn, noticed you're part of this organization as well. Since Mislav didn't reply above, any thoughts on this? Don't mean to step on anyone's toes, though – if Mislav makes these calls I don't want to bypass him :)

@mislav
Copy link
Contributor

mislav commented Jul 9, 2015

The message should be better. Although, as I said, none of the middleware will actually work without an adapter. So, improving the error message specifically for this middleware leaves other cryptic failures in other middleware.

The proper solution should be that Faraday aborts early when a Connection object doesn't include an adapter in the stack. Please subscribe to one of #47, #121, #170

@henrik
Copy link

henrik commented Jul 9, 2015

@mislav Aha, thank you for clarifying!

I think I'll try to make a pull request just to improve the error message from logger – it seems like it could be a fairly simple change that would improve on a common gotcha. Hope that's OK.

I might look into the proper fix also, but it seems more challenging, so I might not have the time or skill to achieve that.

henrik added a commit to henrik/faraday that referenced this issue Jul 9, 2015
The current error message has confused people: lostisland#317

As discussed there, there are better longer-term solutions, but no one has implemented those yet.
manmartinez added a commit to rocketsofawesome/newgistics-ruby that referenced this issue Oct 11, 2017
When adding middleware to the faraday connection we need to
be explicit about which adapter we'll use, otherwise no HTTP
requests are actually made.

Sadly, Faraday doesn't raise an exception if the adapter is
missing and you end up debugging cryptic error messages raised
from the middleware :pandaface:

For more information see:
lostisland/faraday#317
manmartinez added a commit to rocketsofawesome/newgistics-ruby that referenced this issue Oct 11, 2017
When adding middleware to the faraday connection we need to
be explicit about which adapter we'll use, otherwise no HTTP
requests are actually made.

Sadly, Faraday doesn't raise an exception if the adapter is
missing and you end up debugging cryptic error messages raised
from the middleware 🐼

For more information see:
lostisland/faraday#317
manmartinez added a commit to rocketsofawesome/newgistics-ruby that referenced this issue Oct 11, 2017
When adding middleware to the faraday connection we need to
be explicit about which adapter we'll use, otherwise no HTTP
requests are actually made.

Sadly, Faraday doesn't raise an exception if the adapter is
missing and you end up debugging cryptic error messages raised
from the middleware 🐼

For more information see:
lostisland/faraday#317
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants