Unexpected change to Faraday::Error#initialize signature in 0.17.1

[manuelmeurer]

Basic Info

• Faraday Version: 0.17.0 -> 0.17.1
• Ruby Version: n/a

Issue description

From 0.17.0 to 0.17.1, the signature of Faraday::Error#initialize changed.
In 0.17.0, Faraday::Error simply inherited from StandardError, and Faraday:: ClientError inherited from Faraday::Error AND added a custom initialize method that accepts a response parameter.
Now in 0.17.1, Faraday::Error still inherits from StandardError, but also has the custom initialize method that used to be in Faraday:: ClientError.
This change was done in 6e746fc and doesn't seem to have anything to do with the other changes in the commit.

It might make sense to be able to set the response for Faraday::Error (and all errors inheriting from it), not just Faraday:: ClientError, but I would have expected this to cause a new minor or even major version release, not just a new patch release.

The change creates a problem in https://github.com/slack-ruby/slack-ruby-client:
slack-ruby/slack-ruby-client#305 (comment)

Would you accept a PR that reverts this behavior and release 0.17.2?

[iMacTia]

Hi @manuelmeurer, sorry to hear this is causing issues!
We've made some improvements in 1.0 around errors that we wanted to back port in 0.1x as well, but in doing so it seems that we created an incompatibility.

Ideally, we'd like to keep the improvements while maintaining backwards compatibility, so I'll have a look if that's possible.
If that's not the case, then a revert will be the only option.

[iMacTia]

@manuelmeurer I had a look at slack-ruby/slack-ruby-client#305 and to be fair, the problem to me seems that you've not followed the best practice on subclassing.
I might be wrong because I haven't actually tested it, but you should initialise the @response after the call to super.
That is not only the correct way to run initialisation in subclasses, but it would have set the @response to the correct value.

In our case, we have extended a method (init) which was accepting a single parameter to accept another optional one. So from a OOP point of view we haven't done anything breaking or backwards incompatible.

I'm reluctant at this point to revert our change, hopeful my explanation makes sense to you, but please feel free to come back with any question you might have.

For now I'm closing this issue as per the explanation above.

[manuelmeurer]

@iMacTia Yep, you're absolutely right, assigning @response after calling super is the better approach. Don't know why I didn't see that... :)