Elasticsearch: Improve "jar hell" message

[jordansissel]

Exception in thread "main" java.lang.IllegalStateException: jar hell!

"jar hell" is not an actionable message. As an Elasticsearch plugin author and a user, this message doesn't inform me as to the problem and what action needs to be taken.

I propose new messaging:

There is a bug in this plugin. The problem is that it is providing a java class that is already defined elsewhere, possibly more than once. If you are a user (not the plugin author), you should file a bug on this plugin's project issue tracker.

The following details may be useful in filing a bug report or troubleshooting this:

Class: io.netty.buffer.AbstractByteBuf

Found in the following jars:
jar1: /home/jls/build/elasticsearch-7.0.0-alpha1-SNAPSHOT/plugins/.installing-1419306632585542845/netty-buffer-4.1.13.Final.jar
jar2: /home/jls/build/elasticsearch-7.0.0-alpha1-SNAPSHOT/plugins/.installing-1419306632585542845/netty-all-4.0.4.Final.jar
...

[rjernst]

"jar hell" is a well known term within the Java community. Additionally, I have a couple problems with changing the wording:

• It is not meant for users. If a plugin author ships a version with jarhell, they have much bigger problems. We don't (and shouldn't) try to catch every runtime bug a plugin author might have made and wrap with user messaging.
• Wordy exception messages do more harm than good when debugging (making it hard to find the information necessary to fix the bug). Again, this is meant for developers.
• Our jar hell detection is not only used by plugins. Elasticsearch bootstrap also checks itself, so having the message be plugin specific doesn't make sense.

What I could be convinced to do is tweak/wrap the output from plugin installation, so that if we detect jarhell, we add a message like "The plugin could not be installed because it is internally corrupt." or something along those lines (followed by rethrowing the exception, which would then be printed).

[jordansissel]

"jar hell" is a well known term within the Java community

Hello. I have written Java off-and-on for 15 years. I have worked here for 4 years. I was unfamiliar with the meaning of this term until I filed this ticket yesterday.

So I don't know what "well known" means in your context, but it is not well-known to me.

It is not meant for users. If a plugin author ships a version with jarhell, they have much bigger problems

I am confused. As a user, I received this information. Why deliver a message to a user when you did not intend it?

Wordy exception messages do more harm than good when debugging (making it hard to find the information necessary to fix the bug). Again, this is meant for developers.

I have had success doing what you believe to be harmful in Logstash, fpm, and other places.

Our jar hell detection is not only used by plugins. Elasticsearch bootstrap also checks itself, so having the message be plugin specific doesn't make sense

This is an implementation detail that doesnt seem relevant to me either as a plugin author nor as Elasticsearch user.

[jasontedor]

So I don't know what "well known" means in your context, but it is not well-known to me.

It's definitely an interesting data point given your background and something we should keep in mind in revisiting the messaging here. This is not something that I would expect users to be familiar with, especially as our user base continues to expand. It is a common problem that Java developers face and why there are all sorts of tricks like shading and dependency exclusion, and the primary reason the OSGi effort came in to being. I say this only as additional background, nothing more.

I am confused. As a user, I received this information. Why deliver a message to a user when you did not intend it?

I'm curious, how did you run into this as a user? A plugin that fails JAR hell really should never make it out the door because it can never have been successfully installed on a cluster while the plugin author was doing local development. It's effectively shipping completely broken code that passes compilation but not any basic integration tests (manual or otherwise).

This is an implementation detail that doesnt seem relevant to me either as a plugin author nor as Elasticsearch user.

I think that @rjernst's point is that we can not change the exception message because of its multi-use, but he is open to considering supplying additional wording in the plugin CLI.

[jordansissel]

I'm curious, how did you run into this as a user?

I am doing an experiment with x-pack-elasticsearch. I added a dependency in build.gradle (tealess) which transitively depends on netty-all. Elasticsearch ships with netty-buffer and jar-hell triggers at install time due to the overlapping java class definitions.

As a developer, I did this, gradle assemble and it was successful (no errors). Then, as a user, I installed the plugin and received the "jar hell" error.

A plugin that fails JAR hell really should never make it out the door

There's no JAR hell check to prevent gradle assemble from succeeding, though, so it was easy for me to have this fail.

I think the problem I experience is this:

• As a developer, I can build a plugin successfully that would fail jar-hell check.
• As a user, I can install a plugin that would fail jar-hell check.

Proposal: Can we add a gradle check for this "jar hell" thing? This would prevent a plugin from being shipped at all if it fails this check.
Impact: A user will (under normal circumstances) never receive this jar-hell problem.

I think there's perhaps one other edge case that would cause a user to see this jar-hell error, and this is when there are two 3rd party plugins with the same dependency but on two different versions. (say, httpclient 4.5 vs httpclient 4.5.1). I havent' experimented, but I think this would also trigger the jar-hell error for a user who is installing valid plugins but creates an invalid scenario with two plugins together creating a jar-hell scenario. To me, this indicates we should improve the error message.

Some light proposals:

• Make gradle assemble fail for plugins that would cause jar-hell when loaded in Elasticsearch
• Improve the user messaging to indicate that this is likely not the fault of the user, but a problem with the plugin itself or the environment causing such jar-hell. ?

[rjernst]

As a developer, I did this, gradle assemble and it was successful (no errors).
As a developer, I can build a plugin successfully that would fail jar-hell check.

gradle assemble only compiles and builds the jar. It does no checks. gradle check runs all tests. gradle build is synonymous with gradle assemble check. So if you want to build and test, use gradle build. This is not elasticsearch specific, it is standard gradle workflow.

Then, as a user, I installed the plugin and received the "jar hell" error.
As a user, I can install a plugin that would fail jar-hell check.

I don't see how the latter statement is true. You tried installing the plugin, and it failed. As @jasontedor said, this can only happen by a plugin author ships a broken plugin and we did not actually install the plugin.

I think there's perhaps one other edge case that would cause a user to see this jar-hell error, and this is when there are two 3rd party plugins with the same dependency but on two different versions.

This is why we do not use transitive dependencies. When using the ES gradle plugin build, a plugin author must be explicit about all dependencies, forcing them to decide on how this conflict is resolved.

Improve the user messaging to indicate that this is likely not the fault of the user

Jar hell is always the fault of the plugin author (unless the user has messed with the plugin installation themselves, but then they essentially become the plugin author). Please see my suggestion again on messaging.

[jordansissel]

This is not elasticsearch specific, it is standard gradle workflow.

I think it is a bug that someone can build an invalid plugin. Citing standard-workflow doesn't do any services to a user (hi!) who is confused by this messaging.

This is why we do not use transitive dependencies

We have an opportunity to enforce this in gradle, do we not? Can we? There seem to be a lot of spoken policies that could be enforced programmatically at build-time.

What I am trying to say is we have tooling that permits invalid plugins to be built and shipped.

Jar hell is always the fault of the plugin author

Pointing fingers may be useful, but the message from the exception does not point fingers. These things you are telling me can be in the message: It's the plugin-author's fault fault, definition of jar hell, transitive dependencies are bad, etc.

You are citing some clear assumptions that make sense to me when you cite them, but our messaging does not cite these assumptions , and our tooling (gradle) does not enforce these assumptions.

I am thankful to be better informed about the policies (no dependencies, assumptions of plugin-author-fault), but I am specifically asking for these assumptions to be announced at the time of the error. If we dislike large error messages, would you accept documentation for this in the Elasticsearch plugin docs and a link to it from the error message?

Notably, googling for "elasticsearch jar hell" is full of folks hitting jar hell and asking questions but no definition (From our docs) about what it means and what action to take to resolve it.

[rjernst]

We have an opportunity to enforce this in gradle, do we not? Can we? There seem to be a lot of spoken policies that could be enforced programmatically at build-time.

We already do check this. It is one of the tasks run by the precommit task, which is run with gradle check. As I said, assemble is by design only meant to build. It does not even build tests. Your argument is equivalent to complaining that gradle jar doesn't run tests. The task is meant to build a jar, not do other things. gradle assemble is meant to assemble the artifacts, not run any checks on them.

[rjernst]

Pointing fingers may be useful, but the message from the exception does not point fingers.

The exception message says exactly what is wrong. What I have proposed is improved messaging in the plugin installer, which is the only place a user could see this, due to a bad plugin being shipped by a plugin author. This exception happening during ES startup is only possible if a user has mucked with the installation, which I do not think we need to improve error messaging for.

[jordansissel]

I would value improved messaging for users.

For authors, I still wish for the tooling to not produce jar-hell'd
packages.

On Wed, Oct 11, 2017 at 12:52 PM Ryan Ernst [e-mail]
wrote:

Pointing fingers may be useful, but the message from the exception does
not point fingers.

The exception message says exactly what is wrong. What I have proposed is
improved messaging in the plugin installation, which is the only place a
user could see this, due to a bad plugin being shipped by a plugin author.
This exception happening during ES startup is only possible if a user has
mucked with the installation, which I do not think we need to improve error
messaging for.

—
You are receiving this because you authored the thread.

Reply to this email directly, view it on GitHub
https://github.com/elastic/elasticsearch/issues/26957#issuecomment-335927744,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAIC6pwj9cNGlEEjZAMFmNszKwNwqnGhks5srRyTgaJpZM4P05Uz
.

[jasontedor]

I added a dependency in build.gradle (tealess) which transitively depends on netty-all. Elasticsearch ships with netty-buffer and jar-hell triggers at install time due to the overlapping java class definitions.

It’s not a conflict with Elasticsearch. The Netty transport is modularized, so sits in a separate classloader from X-Pack. What you’re facing is that X-Pack depends on Netty as well, and that’s why you have JAR hell within your modified X-Pack.

I think there's perhaps one other edge case that would cause a user to see this jar-hell error, and this is when there are two 3rd party plugins with the same dependency but on two different versions. (say, httpclient 4.5 vs httpclient 4.5.1).

This is not a problem. Plugins are isolated into separate classloaders and the JAR hell checks are per classloader (including the parent classloaders). Reiterating on the above: Netty transport and X-Pack are in sibling classloaders, and isolated.

[jasontedor]

If we dislike large error messages, would you accept documentation for this in the Elasticsearch plugin docs and a link to it from the error message?

We would accept documentation for this in the Elasticsearch plugin docs, I think that's a great suggestion. We would not accept a link to it from the error message, we do not want links in our software that could break.

[jrodewig]

[docs issue triage]

[williamrandolph]

It looks like we still don't have any user-facing documentation on the "jar hell" message, and could add it to the "Help for Plugin Authors"

[jasontedor]

Is this large enough of a problem that we need to take on any effort to solve it? I haven't seen complaints about our JAR hell check in a long time.

[williamrandolph]

@jasontedor I haven't seen any either.

Since the demand for this improvement seems to be low and no one has taken it on despite the "help wanted" tag, I will close the issue.

If you are a user and would like to see action on this, please feel free to re-open with a comment.