Apache http client fix exception handling #417
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mar-kolya
force-pushed
the
mar-kolya/apache-http-client-fix-exception-handling
branch
3 times, most recently
from
64e61af
to
020a168
Compare
tylerbenson
suggested changes
Jul 31, 2018
| * @see net.bytebuddy.matcher.HasSuperTypeMatcher | ||
| */ | ||
| @HashCodeAndEqualsPlugin.Enhance | ||
| public static class SafeHasSuperTypeMatcher<T extends TypeDescription> |
There was a problem hiding this comment.
Can you include in your comment how this is different from the SuperTypeMatcher that is built into byte buddy?
Also describe how this is different from using failSafe(hasSuperType(...)).
There was a problem hiding this comment.
should be fixed now
mar-kolya
force-pushed
the
mar-kolya/apache-http-client-fix-exception-handling
branch
2 times, most recently
from
5b5a4dd
to
86af4fe
Compare
mar-kolya
force-pushed
the
mar-kolya/apache-http-client-fix-exception-handling
branch
from
86af4fe
to
982d346
Compare
mar-kolya
changed the title
mar-kolya
force-pushed
the
mar-kolya/apache-http-client-fix-exception-handling
branch
from
982d346
to
a4238aa
Compare
tylerbenson
approved these changes
Jul 31, 2018
This was referenced Jul 31, 2018
|
I tested this branch using the sample app @htmldoug provided in reference to #410 and it seems to solve the problem in a better way than what I did. I've fixed my pr #411 to revert the part that this PR fixes. |
gary-huang
approved these changes
Jul 31, 2018
| */ | ||
| public static <T extends TypeDescription> ElementMatcher.Junction<T> safeHasSuperType( | ||
| final ElementMatcher<? super TypeDescription> matcher) { | ||
| // return hasSuperType(matcher); |
There was a problem hiding this comment.
Is this line not needed?
There was a problem hiding this comment.
Good catch! Removed.
realark
suggested changes
Jul 31, 2018
This makes sure no classes are loaded during instrumentation transformation which allows us to safely instrument depndent classes.
This allows ByteBuddy to properly find classes injected into the agent. Thanks @realark for providing a fix!
It should not be necessary after we jave fixed class location issue for ByteBuddy
Properly open and close outer span in multi-request cases
The idea is to just 'trim' type hierarchy 'up-trees' that we cannot resolve dring instrumentation instead of failing to instrument completely.
We not longer need it since our instrumentation can handle underlying loading problem.
`isSubType` may fail on certain class lookup problems, even on classes unrelated to given instrumentation, preventing instrumentation from being applied.
Newer ByteBuddy api simplifies things.
It looks like they fail tests from time to time
mar-kolya
force-pushed
the
mar-kolya/apache-http-client-fix-exception-handling
branch
from
a4238aa
to
bb2126b
Compare
realark
approved these changes
Aug 1, 2018
mar-kolya
deleted the
mar-kolya/apache-http-client-fix-exception-handling
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should improve how 'hard' cases like redirects and errors are handled by apache http client instrumentation.