Skip to content

Move Faraday middlewares register to an initializer

🐼 Context

In !50750 (merged), the Container Registry client observability has been improved by using a custom Faraday middleware.

This middleware is registered using this class. Reasons explained here.

For a reason I don't clearly see, this commit broke that because ::Gitlab::Faraday is not eager loaded anymore.

To illustrate this point, here is what happens before bc05f258:

[1] pry(main)> Faraday::Request.lookup_middleware(:gitlab_error_callback)
=> Gitlab::Faraday::ErrorCallback

Our custom middleware is properly registered and Faraday can look it up.

Here is currently the situation on master:

Faraday::Request.lookup_middleware(:gitlab_error_callback)
Faraday::Error: :gitlab_error_callback is not registered on Faraday::Request
from /Users/david/.asdf/installs/ruby/2.7.2/lib/ruby/gems/2.7.0/gems/faraday-1.0.1/lib/faraday/middleware_registry.rb:91:in `lookup_middleware'

💥

Note that this does not affect production environments in any way, as all the classes are eager loaded on these.

I'm not sure why bc05f258 breaks the class loading. This commit seems to change how "models" are loaded and our custom middleware is loaded from a class that is in lib. lib should be eager loaded all the times. I'm not sure what is happening 🤷

🔭 What does this MR do?

  • Register the Faraday middleware from an initializer file instead of a class file.

I don't think that a changelog entry is need here. This is Stuff that should Just Work .

🖼 Screenshots (strongly suggested)

With this MR:

Faraday::Request.lookup_middleware(:gitlab_error_callback)
=> Gitlab::Faraday::ErrorCallback

🎉

Does this MR meet the acceptance criteria?

🔩 Conformity

🎠 Availability and Testing

🔒 Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team

Merge request reports