Faraday::Error::ClientError deprecation warning is disruptive

[DannyBen]

Displaying deprecation warnings to STDOUT/STDERR is very disruptive.

Although it is "just a warning", we need to consider that in some cases, these warnings break CI (when checking for expected output) and in other cases, displaying a warning to the end user of a CLI gem is undesirable.

It causes people who are not even using Faraday directly, to waste time on trying to find workarounds, and depend on the offending package maintainer to release an update.

I would like to suggest avoiding this practice in the future, and instead, let the magic of semantic versioning do what it was designed to do. I would even go further and suggest to release an updated version without this warning.

[iMacTia]

Have you considered telling the logger to skip warnings and just log errors in your CI?

[DannyBen]

You should not treat your libraries as if they are only used by developers. Especially when it is such a big and widely used library. At the end of the dependency chain, there are users, who end up seeing useless deprecation warnings, and think that my tool is broken.

Runtime is user-land, not developer-land. Never disrupt runtime.

Forget the CI case, I am not sure why you picked that aspect of my issue to respond to of all problems. This is a development warning, should stay in development land.

[iMacTia]

OK, I wasn't trying to minimise the issue, but according to your initial question it seems like warning logs have no meaning at all. I'd be interested to know in which case you'd use one?

I was just pointing out that warning has been defined as standalone log level specifically for these cases, they're logged for developers, not end users.
Moreover, "end users" should not see warnings or have access to log files. I don't know what sort of application you're using Faraday in, but if it's Web/UI based like normal people are used to, then warnings should not be visible to them.
User usually only see error or fatal logs.

If that's not the case for you, then happy to explore further how your specific use case works, but please don't assume everyone is having your same issue, otherwise we'd be seeing many more issues like this one

[DannyBen]

Well, one of us might be missing some piece of information.

I am not talking about logs, of course logs are usually considered developer-land.

I am talking about STDOUT / STDERR.

I was specifically exposed to this problem by using octokit. See this issue:
octokit/octokit.rb#1170

and you can tell by the not so small amount of emojis it got stuck to it, that I am not the only one suffering from this problem. The problem was attributed to Faraday, and the solution, to downgrade Faraday until Octokit releases a new version (and here is another dude who had to do the same).

Since you are talking about logs, I am now not sure if this is directly a Faraday issue, or an Octokit issue... (despite the fact that downgrading Farady solved the issue).

[technoweenie]

Thanks for your feedback. The warnings only really matter if you're trying to upgrade your application or gem (such as Octokit) from Faraday 0.17.x to 1.0.

We're going to look into making these warnings opt-in. Here's the behavior that I'm shooting for:

• v0.17.2 (0.1x)
  • Default: no warnings
  • Run with FARADAY_DEPRECATIONS=1: warnings
• v1.0
  • Default: ruby exceptions!

If someone is trying to upgrade to v1.0, you should fix all warnings reported by FARADAY_DEPRECATIONS=1, and THEN try upgrading the gem version.

[kyrofa]

This seems to be an issue again on the jump from v1 to v2. I'm getting a bunch of these:

WARNING: `Faraday::Connection#basic_auth` is deprecated; it will be removed in version 2.0.
While initializing your connection, use `#request(:basic_auth, ...)` instead.
See https://lostisland.github.io/faraday/middleware/authentication for more usage info.
WARNING: `Faraday::Connection#basic_auth` is deprecated; it will be removed in version 2.0.
[...]

Did you move away from the FARADAY_DEPRECATE idea? To be clear, I don't even directly depend on faraday-- it comes through another dependency.

[iMacTia]

@kyrofa thanks for raising this. I agree that our 1.0 -> 2.0 upgrade warning behaviour is not consistent with what we did in the past. Unfortunately, the Faraday::Deprecate class was never ported to 1.0 so I forgot we could have used that.

I'll open a new issue from your comment, and get to it when I get a chance, but if someone else has some time to work on it, any help would also be appreciated 🙏

[iMacTia]

@kyrofa FYI the warnings were fixed in #1438 thanks to @hyuraku 🎉