-
Notifications
You must be signed in to change notification settings - Fork 132
faraday: Add logging formatter signatures #1037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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') |
| 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 |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -225,6 +225,12 @@ module Faraday | |||||
| end | ||||||
|
|
||||||
| class Response | ||||||
| class Logger < Middleware | ||||||
| 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 | ||||||
|
|
@@ -264,6 +270,42 @@ module Faraday | |||||
| def close: () -> void | ||||||
| end | ||||||
|
|
||||||
| module Logging | ||||||
| class Formatter | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think these return types should be def request: (Env env) -> void
def response: (Env env) -> void
def exception: (untyped exc) -> void
def filter: (untyped filter_word, untyped filter_replacement) -> voidThe test signature in _test/test_logging_formatter.rbs should probably use void as well, since it documents the expected override contract for custom formatters.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implemented in |
||||||
| 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 | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The implementation also has a private Could you add it alongside def log_errors?: () -> boolish
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added to |
||||||
|
|
||||||
| def dump_headers: (untyped headers) -> String | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
return if headers.nil?So this should be
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added signature overload in |
||||||
| 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 | ||||||
|
|
@@ -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 | ||||||
| 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')) |
| 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 |
| 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Faraday::Response::Loggeralso defineson_error(exc)in the implementation, but it is missing from this signature.Could you add it here?
There was a problem hiding this comment.
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