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

Explicitly require to define FaradayMiddleware::RedirectLimitReached #233

Merged

Conversation

luvtechno
Copy link
Contributor

This change fixes an error of NameError: uninitialized constant FaradayMiddleware::RedirectLimitReached.

The issue is that MetaInspector::Request uses FaradayMiddleware::RedirectLimitReached as below, which can result in NameError: uninitialized constant when FaradayMiddleware::FollowRedirects is not loaded.

rescue Faraday::Error::ConnectionFailed, Faraday::SSLError, URI::InvalidURIError, FaradayMiddleware::RedirectLimitReached => e

This happens because faraday_middleware auto-loads FaradayMiddleware::FollowRedirects class
as below, which also defines FaradayMiddleware::RedirectLimitReached error class in the same file.

https://github.com/lostisland/faraday_middleware/blob/895a55511699950c535d29291aa8e9dc70b7b6d4/lib/faraday_middleware.rb#L18

Explicit requiring of the file will fix the problem.

@jaimeiniesta
Copy link
Owner

Thanks! The PR looks fine but I would like to understand how we didn't catch this error before.

How can I reproduce it?

@luvtechno
Copy link
Contributor Author

how we didn't catch this error before.

Yes, this is strange part. I also wonder why. Either 1) everyone is using allow_redirections option with metainspector, or 2) everyone who use metainspector always use FaradayMiddleware::FollowRedirects middleware, i guess 🤔

How can I reproduce it?

Here is the repro. This is the real example we came into this error.

$ irb
irb(main):001:0> require 'metainspector'
=> true
irb(main):002:0> url = "https://www.magata.net/memo/index.php?Blob%A5%C7%A1%BC%A5%BF%A4%F2%B0%B7%A4%A6%28Oracle%29"
=> "https://www.magata.net/memo/index.php?Blob%A5%C7%A1%BC%A5%BF%A4%F2%B0%B7%A4%A6%28Oracle%29"
irb(main):003:0> MetaInspector.new(url)
NameError: uninitialized constant FaradayMiddleware::RedirectLimitReached

@jaimeiniesta jaimeiniesta merged commit 70a9e25 into jaimeiniesta:master May 15, 2018
@jaimeiniesta
Copy link
Owner

Thanks for the explanation!

I have reproduced the bug, and merged the fix. Now, however I run into a different error for that URL, ArgumentError: invalid byte sequence in US-ASCII.

Released in 5.4.3

@luvtechno
Copy link
Contributor Author

Thank you!

Now, however I run into a different error for that URL, ArgumentError: invalid byte sequence in US-ASCII.

Character encoding issue always bites me... Anyways that's a separate issue and thanks for releasing it!

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

Successfully merging this pull request may close these issues.

2 participants