Friendlier handling of DeploymentHandshakeException from CLI in -webSocket mode

[jglick]

If the CLI client gets as far as authenticating the connection and establishing the WebSocket upgrade, then the remaining error handling is fairly well defined

jenkins/core/src/main/java/hudson/cli/CLICommand.java

Lines 223 to 237 in 48f7a0c

	* Exit code from the CLI command execution
	* <table>
	* <caption>Jenkins standard exit codes from CLI</caption>
	* <tr><th>Code</th><th>Definition</th></tr>
	* <tr><td>0</td><td>everything went well.</td></tr>
	* <tr><td>1</td><td>further unspecified exception is thrown while performing the command.</td></tr>
	* <tr><td>2</td><td>{@link CmdLineException} is thrown while performing the command.</td></tr>
	* <tr><td>3</td><td>{@link IllegalArgumentException} is thrown while performing the command.</td></tr>
	* <tr><td>4</td><td>{@link IllegalStateException} is thrown while performing the command.</td></tr>
	* <tr><td>5</td><td>{@link AbortException} is thrown while performing the command.</td></tr>
	* <tr><td>6</td><td>{@link AccessDeniedException} is thrown while performing the command.</td></tr>
	* <tr><td>7</td><td>{@link BadCredentialsException} is thrown while performing the command.</td></tr>
	* <tr><td>8-15</td><td>are reserved for future usage.</td></tr>
	* <tr><td>16+</td><td>a custom CLI exit error code (meaning defined by the CLI command itself)</td></tr>
	* </table>

and tested. However I noticed that lower-level errors making the connection were not reported gracefully by the client. (They are not reported very gracefully in the older -http mode either, but as of #7605 that is not used by default.)

Originally noticed in the context of a proprietary plugin sending a 401 response under certain circumstances, but in fact seems applicable also to the more mundane scenaio of a mistyped API token.

Note that the response body is not exposed by the Tyrus client (even importing implementation classes), so this cannot be displayed, only the status code and headers. I actually prototyped a rewrite to the JDK’s built-in WebSocket client, which offers access to the response, but the body is still empty. (It may be a good idea to do this rewrite anyway, to avoid an external dep and bloat in jenkins-cli.jar.)

Testing done

Without patch:

io.jenkins.cli.shaded.org.glassfish.tyrus.client.exception.DeploymentHandshakeException: Handshake error.
	at io.jenkins.cli.shaded.org.glassfish.tyrus.client.exception.Exceptions.deploymentException(Exceptions.java:38)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.client.ClientManager$3$1.run(ClientManager.java:622)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.client.ClientManager$3.run(ClientManager.java:657)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.client.ClientManager$SameThreadExecutorService.execute(ClientManager.java:810)
	at java.base/java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:123)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.client.ClientManager.connectToServer(ClientManager.java:460)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.client.ClientManager.lambda$connectToServer$2(ClientManager.java:313)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.client.ClientManager.tryCatchInterruptedExecutionEx(ClientManager.java:324)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.client.ClientManager.connectToServer(ClientManager.java:313)
	at hudson.cli.CLI.webSocketConnection(CLI.java:360)
	at hudson.cli.CLI._main(CLI.java:313)
	at hudson.cli.CLI.main(CLI.java:101)
Caused by: io.jenkins.cli.shaded.org.glassfish.tyrus.client.auth.AuthenticationException: Credentials are missing.
	at io.jenkins.cli.shaded.org.glassfish.tyrus.client.auth.BasicAuthenticator.generateAuthorizationHeader(BasicAuthenticator.java:39)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.client.auth.BasicAuthenticator.generateAuthorizationHeader(BasicAuthenticator.java:34)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.client.TyrusClientEngine.createUpgradeRequest(TyrusClientEngine.java:214)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.container.jdk.client.JdkClientContainer$1.call(JdkClientContainer.java:117)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.container.jdk.client.JdkClientContainer$1.call(JdkClientContainer.java:108)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.container.jdk.client.ClientFilter.processRead(ClientFilter.java:183)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.container.jdk.client.Filter.onRead(Filter.java:111)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.container.jdk.client.Filter.onRead(Filter.java:113)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.container.jdk.client.Filter.onRead(Filter.java:113)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.container.jdk.client.TransportFilter$4.completed(TransportFilter.java:295)
	at io.jenkins.cli.shaded.org.glassfish.tyrus.container.jdk.client.TransportFilter$4.completed(TransportFilter.java:279)
	at java.base/sun.nio.ch.Invoker.invokeUnchecked(Invoker.java:129)
	at java.base/sun.nio.ch.UnixAsynchronousSocketChannelImpl.finishRead(UnixAsynchronousSocketChannelImpl.java:447)
	at java.base/sun.nio.ch.UnixAsynchronousSocketChannelImpl.finish(UnixAsynchronousSocketChannelImpl.java:195)
	at java.base/sun.nio.ch.UnixAsynchronousSocketChannelImpl.onEvent(UnixAsynchronousSocketChannelImpl.java:217)
	at java.base/sun.nio.ch.EPollPort$EventHandlerTask.run(EPollPort.java:306)
	at java.base/sun.nio.ch.AsynchronousChannelGroupImpl$1.run(AsynchronousChannelGroupImpl.java:113)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:840)

With:

CLI handshake failed with status code 401
Cache-Control: must-revalidate, no-cache, no-store
Content-Length: 456
Content-Type: text/html;charset=iso-8859-1
Date: Thu, 08 Aug 2024 21:20:07 GMT
Server: Jetty(10.0.22)
Set-Cookie: remember-me=; Path=/jenkins; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0
WWW-Authenticate: Basic realm="Jenkins"
X-Content-Type-Options: nosniff

Proposed changelog entries

• Better display HTTP handshake errors (such as authentication issues) from the CLI in -webSocket mode.

Before the changes are marked as ready-for-merge:

Maintainer checklist

Preview Give feedback

1. There are at least two (2) approvals for the pull request and no outstanding requests for change.
2. Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
3. Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
4. Proper changelog labels are set so that the changelog can be generated automatically.
5. If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
6. If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

[timja]

/label ready-for-merge

---

This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

[basil]

@jglick I have been seeing this test flake on Windows ever since this PR was merged:

https://ci.jenkins.io/job/Core/job/jenkins/job/master/6468/

The latest build has the same flake:

https://ci.jenkins.io/job/Core/job/jenkins/job/master/6479/

Can you please look at this before tomorrow's weekly to release to ensure that this flaky test doesn't hold up the release?

[basil]

FYI I can actually reproduce the failure on my Linux system, but not every time. Sometimes the new test passes, and sometimes the new test fails.

[basil]

I disabled the test in #9687 to avoid disrupting tomorrow's weekly release.

[basil]

I can't quite tell if this PR has any negative effects on the production code or not, so I just disabled the test. But if there are negative effects on the production code, we might want to revert the non-test changes in this PR as well.

[jglick]

I think the NPE is just something that would have confused the error reporting on a failed request to begin with, though I never tracked down the root bug in Tyrus.