Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions gems/faraday/2.5/_test/test_logging_formatter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
class MyFormatter < Faraday::Logging::Formatter
def request(env)
# Build a custom message using `env`
info('Request') { 'Sending Request' }
end

def response(env)
# Build a custom message using `env`
info('Response') { 'Response Received' }
end

def exception(exc)
# Build a custom message using `exc`
info('Error') { 'Error Raised' }
end
end

conn = Faraday.new(url: 'http:///example.com') do |faraday|
faraday.response :logger, nil, formatter: MyFormatter
end

conn.get('/abc')
5 changes: 5 additions & 0 deletions gems/faraday/2.5/_test/test_logging_formatter.rbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class MyFormatter < Faraday::Logging::Formatter
def request: (Faraday::Env env) -> void
def response: (Faraday::Env env) -> void
def exception: (untyped exc) -> void
end
45 changes: 45 additions & 0 deletions gems/faraday/2.5/faraday.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,12 @@ module Faraday
end

class Response
class Logger < Middleware
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Faraday::Response::Logger also defines on_error(exc) in the implementation, but it is missing from this signature.
Could you add it here?

def on_error: (untyped exc) -> void

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in gems/faraday/2.7/faraday-2.7.rbs

def initialize: (untyped app, ?untyped logger, ?::Hash[Symbol, untyped] options) ?{ (Logging::Formatter formatter) -> void } -> void
def call: (Env env) -> untyped
def on_complete: (Env env) -> void
end

def status: () -> Integer
def headers: () -> untyped
def body: () -> String
Expand Down Expand Up @@ -264,6 +270,42 @@ module Faraday
def close: () -> void
end

module Logging
class Formatter
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these return types should be void rather than untyped.
In the actual implementation, request, response, exception, and filter are side-effect-oriented methods: they write logs or update the formatter's filter list, and their return values are not part of the public contract.

def request: (Env env) -> void
def response: (Env env) -> void
def exception: (untyped exc) -> void
def filter: (untyped filter_word, untyped filter_replacement) -> void

The test signature in _test/test_logging_formatter.rbs should probably use void as well, since it documents the expected override contract for custom formatters.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in request, response, filter changes in gems/faraday/2.5. Addressed exception in gems/faraday/2.7/faraday-2.7.rbs.

extend Forwardable

DEFAULT_OPTIONS: ::Hash[Symbol, untyped]

@logger: untyped
@options: ::Hash[Symbol, untyped]
@filter: ::Array[[untyped, untyped]]

def initialize: (logger: untyped, options: ::Hash[Symbol, untyped]) -> void

def debug: (?untyped progname) ?{ () -> untyped } -> untyped
def info: (?untyped progname) ?{ () -> untyped } -> untyped
def warn: (?untyped progname) ?{ () -> untyped } -> untyped
def error: (?untyped progname) ?{ () -> untyped } -> untyped
def fatal: (?untyped progname) ?{ () -> untyped } -> untyped

def request: (Env env) -> void
def response: (Env env) -> void
def filter: (untyped filter_word, untyped filter_replacement) -> void

private
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation also has a private log_errors? method, which is used by exception, but it is missing from the RBS.

Could you add it alongside log_headers? and log_body??

def log_errors?: () -> boolish

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to gems/faraday/2.7/faraday-2.7.rbs


def dump_headers: (untyped headers) -> String
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dump_headers can return nil when headers is nil in the implementation:

return if headers.nil?

So this should be String? rather than String.

Suggested change
def dump_headers: (untyped headers) -> String
def dump_headers: (untyped headers) -> String?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added signature overload in gems/faraday/2.7/faraday-2.7.rbs

def dump_body: (untyped body) -> String
def pretty_inspect: (untyped body) -> String
def log_headers?: (String | Symbol type) -> boolish
def log_body?: (String | Symbol type) -> boolish
def apply_filters: (untyped output) -> String
def log_level: () -> Symbol
def log_headers: (String type, untyped headers) -> untyped
def log_body: (String type, untyped body) -> untyped
end
end

module MiddlewareRegistry
def registered_middleware: () -> Hash[Symbol, untyped]
def register_middleware: (**untyped mappings) -> void
Expand Down Expand Up @@ -329,5 +371,8 @@ module Faraday
attr_accessor response_body: untyped

alias body request_body

def []: (untyped key) -> untyped
def []=: (untyped key, untyped value) -> untyped
end
end
22 changes: 22 additions & 0 deletions gems/faraday/2.7/_test/test_logging_formatter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
class MyFormatter < Faraday::Logging::Formatter
def exception(exc)
# Build a custom message using `exc`
info('Error') { 'Error Raised' }
end

def dump_nil_headers
dump_headers(nil)
end
end

conn = Faraday.new(url: 'http:///example.com') do |faraday|
faraday.response :logger, nil, formatter: MyFormatter, errors: true
end

conn.get('/abc')

formatter = MyFormatter.new(logger: Object.new, options: {})
formatter.dump_nil_headers

logger = Faraday::Response::Logger.new(Object.new)
logger.on_error(StandardError.new('boom'))
4 changes: 4 additions & 0 deletions gems/faraday/2.7/_test/test_logging_formatter.rbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class MyFormatter < Faraday::Logging::Formatter
def exception: (untyped exc) -> void
def dump_nil_headers: () -> nil
end
18 changes: 18 additions & 0 deletions gems/faraday/2.7/faraday-2.7.rbs
Original file line number Diff line number Diff line change
@@ -1,4 +1,22 @@
module Faraday
class TooManyRequestsError < ClientError
end

class Response
class Logger
def on_error: (untyped exc) -> void
end
end

module Logging
class Formatter
def exception: (untyped exc) -> void

private

def dump_headers: (nil headers) -> nil
| ...
def log_errors?: () -> boolish
end
end
end
Loading