Skip to content

Conversation

@tbalthazar
Copy link
Contributor

Test the response status is correctly traced. Also guard against nil
response/status, which might happen if the middleware is inserted into
Excon before the Excon::Middleware::ResponseParser.

Test the response status is correctly traced.  Also guard against nil
response/status, which might happen if the middleware is inserted into
Excon before the Excon::Middleware::ResponseParser.
Copy link
Member

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

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

LGTM

end

def response_status(datum)
return nil unless datum[:response] && datum[:response][:status]
Copy link
Collaborator

@jcarres-mdsol jcarres-mdsol Mar 21, 2017

Choose a reason for hiding this comment

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

In this case I'd write this slightly simpler:

datum[:response] && datum[:response][:status] && datum[:response][:status].to_s

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point!

@jcarres-mdsol jcarres-mdsol merged commit 0ae79ca into openzipkin:master Mar 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants