Jeffjo/fix excon error handler #426
Conversation
jeffjo
force-pushed
the
jeffjo/fix-excon-error-handler
branch
2 times, most recently
from
9061e22
to
29ed36b
Compare
| handle_response(d) | ||
| end | ||
| handle_response(datum) | ||
| @stack.error_call(datum) |
There was a problem hiding this comment.
I see, we have to handle_response before calling the stack for error_call?
There was a problem hiding this comment.
Yes - see commit message for a more detailed explanation. There's also evidence in the Excon::Middleware::Base class indicating that this is the order it should be:
https://github.com/excon/excon/blob/master/lib/excon/middlewares/base.rb#L11
ddtrace.gemspec
Outdated
| @@ -35,6 +35,7 @@ EOS | |||
| spec.add_dependency 'msgpack' | |||
|
|
|||
| spec.add_development_dependency 'rake', '~> 10.5' | |||
| spec.add_development_dependency 'excon', '~> 0.62.0' | |||
There was a problem hiding this comment.
I don't think we should add this here. Excon is considered a "contrib", and we load these kind of external dependencies via Appraisals, which are used to drive tests. Development dependencies are for the gem itself.
|
Minor feedback, but looking good otherwise! Since this is a bugfix, I suggest rebasing this against |
delner
added
bug
The `Connection` class raises an error in its `error_call` method when there is a timeout. Since the `Connection` class comes after this middleware in the chain, this resulted in the span never being closed. Now, the span is closed before passing control to the rest of the middleware chain.
This test ensures that the Excon tracing middleware appears before the Idempotent middleware so that a span will be created for every retried request.
jeffjo
force-pushed
the
jeffjo/fix-excon-error-handler
branch
from
29ed36b
to
b778104
Compare
|
@delner I don't see the excon instrumentation on master. Should I rebase off a different branch? |
|
@jeffjo Oh, I'm sorry, Excon is new to |
See commit message for details. This manifested as dropped traces due to unclosed spans when a timeout occurs while making a request