Fix for OAuth2::Error trying to encode non-encodable objects

[jhmoore]

We are pretty interested in seeing 2.0.0 released so we can unfork ourselves -- I was looking down the list of remaining items in https://github.com/oauth-xx/oauth2/milestone/1 and thought I might try to pitch in.

This is a replacement for #261 which looks dead. The issue is trying to call encode on something which does not have an encode method. However in trying to test (drive!) a fix for it, it seemed like there were no tests present at all for the OAuth2::Error class. I created a new spec file and did my best to backfill specs for the already-existing functionality (of which there is quite a bit tacked on to the inherited StandardError behavior).

The GitHub diff of the error class itself looks kind of weird because of indentation, but a brief rundown of changes:

• Made #error_message private. Nothing outside this file talks to this method, and it sure seems like a "generate a formatted string for the error message" internal thing.
• Renamed some locals to make me less nervous (e.g. error_message inside of def error_message, message inside a class descending from StandardError).
• Made #initialize not rely on a local being defined only inside an if branch. (Haha I know it works because Ruby is maaaagic, but y'know)
• If error_description is present but not error, avoid displaying an errant ": ".
• If a formatted error description is not nil but is blank, avoid displaying a blank newline.
• Backfilled specs for encoding and multi-line message generation.

Also: I was unable to determine why @code and @description are set and made into attr_readers -- nothing outside the file seems to talk to those things either. However, folks using this error in their implementations certainly could be relying on being able to call error.code or whatever. I elected to leave them there (and test that they are there), but open to suggestions on that front.

[coveralls]

Pull Request Test Coverage Report for Build 615

• 0 of 0 (NaN%) changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at ?%

---

Totals
Change from base Build 602:	0.0%
Covered Lines:
Relevant Lines:	0

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 609

• 0 of 0 (NaN%) changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at ?%

---

Totals
Change from base Build 602:	0.0%
Covered Lines:
Relevant Lines:	0

---

💛 - Coveralls

[jhmoore]

Oh now this is interesting. The Travis builds in Ruby 1.9 and 2.0 are failing . . . . I believe that this behavior may have been broken in Ruby versions < 2.1 for some time but there was just no test to fail for it?

Apparently the behavior of encode changed with 2.1. Confirmed in consoles:

2.0.0-p648 :001 > "\xAD bad!".encode("UTF-8", :invalid => :replace)
 => "\xAD bad!"

2.3.3-p222 :001 > "\xAD bad!".encode("UTF-8", :invalid => :replace)
 => "� bad!" 

The JRuby builds are also failing in the same way as well.

[pboling]

@jhmoore This is something I would vote for using rspec-pending_for to skip those specs on Ruby < 2.1.

Then we can remove the caveat when we drop old Rubies in the following major release, 3.0.

Something like this (untested, and not sure if the version strings for JRuby use MRI version numbers, but I think they do).

it("blah is blah") do
  pending_for(engine: "ruby", versions:%w(1.9.3 2.0.0), reason: "change in behavior of encode at Ruby 2.1")
  pending_for(engine: "jruby", versions: %w(1.8.7 1.9.3 2.0.0), reason: "change in behavior of encode at Ruby 2.1")
  expect("blah").to eq "blah"
end

[jhmoore]

Hmm, looks like the JRuby versions are still running with MRI version numbers passed in.

Also haha 1.9.3 seems unable to even parse the file at all:

Failure/Error: load file
SyntaxError:
  /home/travis/build/oauth-xx/oauth2/spec/oauth2/error_spec.rb:76: invalid multibyte char (US-ASCII)
  /home/travis/build/oauth-xx/oauth2/spec/oauth2/error_spec.rb:76: invalid multibyte char (US-ASCII)
  /home/travis/build/oauth-xx/oauth2/spec/oauth2/error_spec.rb:76: syntax error, unexpected $end, expecting ')'
  ...ss subject.message.include?('??? invalid ???')

[jhmoore]

@pboling looks like that did the trick!

[pboling]

Awesome.

[jhmoore]

@pboling thanks! Is the list in this milestone an accurate list of blockers? (Other than #261 which can be closed out)

[pboling]

@jhmoore AFAIK, yes. There may be other things lurking in issues, but I expect if we can clear those we'll be good to release.

[jhmoore]

Ok cool, thanks. I'll see what I can pick off there.