-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Include meaningful diagnostics when getAtInject fails due to linkage errors #1279
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
(Still trying to figure out if my employer @cloudbees has a corporate CLA or what.) |
@googlebot I signed it! |
1 similar comment
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
While I appreciate the PR, Guice does a lot of reflection. It's not a winning battle to try/catch every place where inconsistent underlying jars/classes can cause linkage errors. |
No, not every place, but this is one place where an error is actually likely and reported terribly. |
Understood. Google lives in monorepo built-from-head world, so this particular kind of pain isn't something I see day-to-day. I'm concerned that papering over a few specific calls is unlikely to continue working over time. Would you be able to add tests, presumably writing a class using ByteBuddy or ASM to emulate the linkage error, so we can validate this catches and reports the problem correctly? If we're going to do this, we should make sure we handle the following cases:
|
Sounds tricky, assuming there are no existing tests of that sort to use as models. If I can find time to work on this, I will try to write up such a test and reopen accordingly (I guess combined with #1283). Thank you for advice. |
Before:
After:
The enriched error message made it possible to identify the JAR (out of hundreds of candidates) which needed to be recompiled against an updated dependency.