Skip to content

Offline tests; fix read timeout, failure handling, PNG answers, secrets in the log - #37

Merged
franklupo merged 2 commits into
masterfrom
fix/failure-handling
Oct 2, 2026
Merged

franklupo merged 2 commits into
masterfrom
fix/failure-handling

Conversation

@franklupo

Copy link
Copy Markdown
Member

What changes

Two things together: the offline tests of the client, and the fixes they made possible.

Tests

  • JUnit tests of the hand-written classes on a local HTTP server that plays the part of Proxmox VE (MockPveServer): RequestTest, LoginTest, TaskTest, GeneratedClientTest, and FailureTest for the cases below. 55 tests, no cluster needed, run by mvn test and so by the build workflow.
  • LiveClusterTest is tagged live and runs only with -P live (it needs PVE_HOST, PVE_API_TOKEN, PVE_TEST_VMID).
  • The old Test.java (a program that needed a cluster, run with the exec plugin) is removed.

Fixed

  • No read timeout: the timeout was applied only to the connection. A node that accepts the connection and then does not answer blocked the call forever, and with it the wait for a task. The timeout is now also the read timeout, and a request that times out gives a Result with status 408 and the reason.
  • A request that got no answer (connection refused, name not resolved, certificate refused) gave status 0 with an empty reason, so the cases could not be told apart. The reason now carries the exception.
  • A success status with a body that is not JSON (the page of a proxy) was a successful Result with no data. It is now status 502 with the start of the body in the reason; an error status is kept.
  • Result.responseInError() and getError() threw NullPointerException when there was no response, which is the case of every failed request above.
  • PNG response type: the body was read as UTF-8 text line by line and encoded again, so the image was corrupted; the request went to /api2/json. The bytes are now returned as they are, the request goes to /api2/png, an error answer keeps its status and errors, and login and task status are always read as JSON.
  • Log at FINER printed the ticket and the CSRF token of the login answer; the log at FINE printed the query string of GET and DELETE requests, which repeats the parameters unmasked. Both are now masked, and the parameters of every method are listed with the secret ones as ****.
  • login threw NullPointerException on an answer without data or without a ticket; it now returns false. The realm is read from the part after the last @.
  • Task id not valid (null, empty, not a UPID): NullPointerException or ArrayIndexOutOfBoundsException. Now a PveResultException before any request.
  • DELETE sends its parameters in the query string.
  • The Content-Length header was set to the number of characters, not of bytes: the line is removed, the JDK sets the header itself.

Changed

  • getApiUrl() follows the response type of the client (/api2/json or /api2/png). New protected getBaseAddress() gives scheme, host and port.
  • Javadoc of login (when it throws, when it returns false), of setTimeout and of getExitStatusTask (null while the task runs) corrected to what the code does.

Not changed: certificate validation stays off by default, as in the other cv4pve clients.

Tests

On the code before the fix the 11 tests of FailureTest all fail (among them: the call to a silent node still running after 5 seconds, the PNG bytes altered, NullPointerException in responseInError, the ticket in the log). After: 55 passed, 0 failed, with Maven and JDK 23.

The answers are simulated: the fixes were not run against a real cluster. The PNG path in particular should be tried on a node.

@franklupo
franklupo merged commit e2fc0fb into master Oct 2, 2026
5 checks passed
@franklupo
franklupo deleted the fix/failure-handling branch October 2, 2026 17:13
@franklupo franklupo mentioned this pull request Oct 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant