SQL: JDBC should validate existance of SQL url

[elasticmachine]

Original comment by @costin:

Trying JDBC against a vanilla Elasticsearch, without X-Pack or SQL returns a 'weird' error:

Caused by: java.sql.SQLException: Server sent bad type [illegal_argument_exception]. Original type was [request [/_xpack/sql] contains unrecognized parameter: [mode]]. [java.lang.IllegalArgumentException: request [/_xpack/sql] contains unrecognized parameter: [mode]
	at org.elasticsearch.rest.BaseRestHandler.handleRequest(BaseRestHandler.java:92)
	at org.elasticsearch.rest.RestController.dispatchRequest(RestController.java:239)
	at org.elasticsearch.rest.RestController.tryAllHandlers(RestController.java:335)
	at org.elasticsearch.rest.RestController.dispatchRequest(RestController.java:173)

Notice that the error complains an obscure mode parameter when in fact the endpoint does not exist and hence the query is handled by the base handler which triggers the exception above.

The driver should be smarter than this and return an appropriate error message.
There are several approaches:

1. validate the connection

Before doing something on a connection do some kind of validation call (HTTP HEAD) or something along those lines. The issue however is that this adds up over time (we keep validating each connection so x2 calls) and it's also fairly light - we only know the endpoint is there, nothing beyond that.

2. recognize the exception pattern

Except the exception and handle it appropriately - this is similar to the security case, where we can look at the body and understand what went wrong without having to waste any extra calls. It's also something we can extend long term for things like not being able to open a connection, etc... (client issues).

My preference would be for 2.
Thoughts?

[elasticmachine]

Original comment by @nik9000:

+1 for a better error message. Probably worth having something for when SQL is disabled or not installed. I think we already have something for when SQL's license isn't valid, but I imagine we don't test the CLI with it so I expect the error message isn't great.

[bleskes]

A nice error message saying xpack needs to be installed is always nicer then a cryptic internal message. FixedItFriday is in favour of implementing.

PS - this specific error is weird - it should complain about a path not being registered.

[tvernum]

PS - this specific error is weird - it should complain about a path not being registered.

I believe it's just hitting RestIndexAction.
https://github.com/elastic/elasticsearch/blob/v6.3.0/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java#L41

The rest controlled doesn't care that _xpack is not a valid index name, so when RestSqlQueryAction isn't registered, a POST /_xpack/sql will end up routing to RestIndexAction.

[tvernum]

In setup-passwords we implemented a similar check that xpack exists, and security is enabled (because those were actual problems that users were having)
https://github.com/elastic/elasticsearch/blob/master/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java#L355

[costin]

Thanks for the pointer @tvernum (I see the familiar JDK HttpUrlConnection being used).
We're likely to end up with something quite similar.

Cheers,

[pendext]

I'd like to pick this issue up.

[astefan]

Master: a7e08f4
6.6.0: 12e1806

[astefan]

Backported to 6.5.x: 8d83b2d