Add a Test Connection button to TestingBot credentials#36
Conversation
Adds a "Test Connection" validate button to the credentials form that calls the TestingBot REST API (getUserInfo) with the entered key/secret and reports whether they authenticate — e.g. "Connection successful — signed in as <email> (<plan> plan)" or a clear error. This is the credential verification Sauce/BrowserStack offer and we lacked. The doVerifyCredentials descriptor method is @post (CSRF-safe), gated to admins or users who can configure the surrounding item, and resolves the secret via Secret.fromString so it works whether the form submits plaintext or the encrypted value. The REST client is closed via try-with-resources.
📝 WalkthroughWalkthroughAdds a credential verification action to the TestingBot Jenkins credentials form. The endpoint checks authorization and inputs, decrypts the secret, queries TestingBot for user details, and returns validation feedback containing the authenticated email and optional plan. ChangesCredential Verification
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant CredentialsForm
participant DescriptorImpl
participant TestingbotREST
CredentialsForm->>DescriptorImpl: Submit key and secret
DescriptorImpl->>DescriptorImpl: Check permission and validate inputs
DescriptorImpl->>TestingbotREST: Fetch user info
TestingbotREST-->>DescriptorImpl: Return email and optional plan
DescriptorImpl-->>CredentialsForm: Return FormValidation result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/testingbot/TestingBotCredentials.java`:
- Around line 337-339: Update the authentication handling in
TestingBotCredentials to catch only the specific checked I/O or REST API
exception types declared by the client call, while preserving the existing
user-facing authentication error for those failures. Do not catch broad
Exception; allow unexpected runtime exceptions to propagate or handle them
distinctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: dc0457f0-35e8-4af0-a708-0bdbeaefda13
📒 Files selected for processing (2)
src/main/java/testingbot/TestingBotCredentials.javasrc/main/resources/testingbot/TestingBotCredentials/credentials.jelly
… on #36) doVerifyCredentials caught Exception, masking unexpected bugs. The testingbotrest client signals failures via two specific (unchecked) types — TestingbotUnauthorized Exception (bad key/secret) and TestingbotApiException (which wraps network/parse IOException) — so catch exactly those and let any other runtime exception propagate. Note: getUserInfo()/close() declare no checked exceptions, so narrowing to checked types (as literally worded) isn't possible; these typed unchecked exceptions are the client's declared failure signals.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/testingbot/TestingBotCredentials.java (1)
332-338: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDon't gate credential verification on
user.getEmail().GET /v1/userreturns account details like name and plan, not a guaranteed email field, so valid credentials can be rejected here. Treat a non-null user payload as success or switch to an endpoint that returns the signed-in email, and add a regression test for the no-email response.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/testingbot/TestingBotCredentials.java` around lines 332 - 338, Update the credential verification logic around rest.getUserInfo() to treat any non-null TestingbotUser payload as successful, removing the user.getEmail() requirement. Preserve the plan information in the success message, handle a missing email without rejecting credentials, and add a regression test covering a valid response with no email.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/testingbot/TestingBotCredentials.java`:
- Around line 339-342: Update the exception handling in the credentials
validation flow around TestingbotUnauthorizedException and
TestingbotApiException: keep unauthorized failures returning the
authentication-failed message, but catch TestingbotApiException separately and
return a generic “TestingBot request failed” validation message without
including e.getMessage() or raw response details.
---
Outside diff comments:
In `@src/main/java/testingbot/TestingBotCredentials.java`:
- Around line 332-338: Update the credential verification logic around
rest.getUserInfo() to treat any non-null TestingbotUser payload as successful,
removing the user.getEmail() requirement. Preserve the plan information in the
success message, handle a missing email without rejecting credentials, and add a
regression test covering a valid response with no email.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: efd5cc89-3d63-4f51-b897-50abfe052969
📒 Files selected for processing (1)
src/main/java/testingbot/TestingBotCredentials.java
| } catch (TestingbotUnauthorizedException | TestingbotApiException e) { | ||
| // The REST client's typed failures: bad key/secret, and network/parse errors (it wraps | ||
| // IOException into TestingbotApiException). Unexpected runtime exceptions propagate. | ||
| return FormValidation.error("TestingBot authentication failed: " + e.getMessage()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Local file outline =="
ast-grep outline src/main/java/testingbot/TestingBotCredentials.java --view expanded || true
echo
echo "== Relevant local lines =="
sed -n '320,350p' src/main/java/testingbot/TestingBotCredentials.java
echo
echo "== Local references to TestingbotApiException/Unauthorized =="
rg -n "Testingbot(ApiException|UnauthorizedException)" src/main/java -S || trueRepository: testingbot/TestingBot-Jenkins-Plugin
Length of output: 3692
🌐 Web query:
TestingbotREST.java TestingbotApiException non-401 HTTP parse IOException raw GitHub
💡 Result:
In the testingbot-java client, the TestingbotApiException is typically raised when the TestingBot API returns a non-2xx HTTP status code that is not handled by specific exception types like TestingbotUnauthorizedException [1]. When you encounter an IOException during the parsing of a non-401 HTTP response, it often indicates that the client received a response body that could not be parsed as expected (e.g., malformed JSON) or that a network-level issue occurred during the request-response cycle [2][1]. To diagnose and resolve this: 1. Verify Credentials: Ensure your TESTINGBOT_KEY and TESTINGBOT_SECRET are correctly set [3][4]. A 401 Unauthorized error specifically points to authentication failures, while other status codes or parsing errors will trigger the general TestingbotApiException [3][1]. 2. Inspect the Raw Response: If the library throws an IOException or TestingbotApiException due to a parsing failure, the underlying cause is often an unexpected response format [2]. Ensure you are not accidentally hitting an HTML error page or a proxy message instead of the expected JSON API response. 3. Check Network/Environment: Network-level issues (such as timeouts or connection resets) can sometimes be misreported if the client attempts to parse an empty or interrupted response stream [5][2]. 4. Library Updates: If you are using an older version of the Java client, consider checking the repository for updates, as newer versions of TestingBot's client libraries have moved toward more robust, typed error hierarchies to better distinguish between transport failures, authentication errors, and API-level errors [5][6]. For debugging purposes, you can often capture the raw response or status code by wrapping the API call in a try-catch block and inspecting the exception object provided by the testingbot-java library [1].
Citations:
- 1: https://github.com/testingbot/testingbot-java
- 2: https://getstream.io/chat/docs/java/exception-handling/
- 3: https://testingbot.com/support/api
- 4: https://github.com/testingbot/app-automate-java
- 5: https://github.com/testingbot/testingbot-php/blob/HEAD/CHANGELOG.md
- 6: https://github.com/testingbot/testingbot-api/releases/tag/v1.2.0
🌐 Web query:
site:github.com/testingbot/testingbot-java TestingbotApiException TestingbotUnauthorizedException TestingbotREST.java getUserInfo
💡 Result:
In the TestingBot Java REST client library, the getUserInfo method is used to retrieve details about the current user account [1]. This method is part of the TestingbotREST class [1]. When interacting with the TestingBot API via this library, methods like getUserInfo can throw specific exceptions to handle API-related errors [1]: 1. TestingbotApiException: This exception is thrown when an API error occurs, and it typically contains the JSON response from the server as a string, which can be useful for debugging [1]. 2. TestingbotUnauthorizedException: This exception is thrown when the API request fails due to invalid authentication credentials (API key or secret) [1]. To use the client, it is recommended to manage resources properly, for instance by using a try-with-resources block since the TestingbotREST class implements Closeable [1]. An example usage of the getUserInfo method is provided below: try (TestingbotREST restApi = new TestingbotREST("key", "secret")) { TestingbotUser user = restApi.getUserInfo; // Use user object... } catch (TestingbotUnauthorizedException e) { // Handle authentication error } catch (TestingbotApiException e) { // Handle API error }
Citations:
🌐 Web query:
"TestingbotApiException" "TestingbotUnauthorizedException" "TestingbotREST.java" GitHub
💡 Result:
The classes TestingbotApiException and TestingbotUnauthorizedException are custom exception types used within the official TestingBot Java client library [1]. This library, available on GitHub as testingbot-java, provides the TestingbotREST class to facilitate interaction with the TestingBot REST API [1][2][3]. According to the library's documentation, all API methods provided by the TestingbotREST class are designed to throw these exceptions when errors occur [1]. Specifically: - TestingbotApiException(String json): Thrown when the API returns an error response [1]. - TestingbotUnauthorizedException: Thrown when an authentication error occurs, typically associated with an invalid API key or secret [1][2]. To integrate this in a Java project, you can add the following dependency (using the version 1.0.10 as referenced in official documentation) [4][5][6]: Maven: com.testingbot testingbotrest 1.0.10 Gradle: implementation 'com.testingbot:testingbotrest:1.0.10' The TestingbotREST class is used to authenticate requests using your TestingBot API key and secret, allowing you to perform actions such as updating test metadata, retrieving test information, or managing browser lists [1][2][3].
Citations:
- 1: https://github.com/testingbot/testingbot-java
- 2: https://testingbot.com/support/api
- 3: https://trust.testingbot.com/support/api
- 4: https://testingbot.com/support/web-automate/selenium/java/testng
- 5: https://testingbot.com/support/web-automate/selenium/java/selenide
- 6: https://testingbot.com/support/web-automate/selenium/java/testng-cucumber
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
cd "$tmpdir"
for url in \
"https://raw.githubusercontent.com/testingbot/testingbot-java/master/src/main/java/com/testingbot/testingbotrest/TestingbotREST.java" \
"https://raw.githubusercontent.com/testingbot/testingbot-java/master/src/main/java/com/testingbot/testingbotrest/TestingbotApiException.java" \
"https://raw.githubusercontent.com/testingbot/testingbot-java/master/src/main/java/com/testingbot/testingbotrest/TestingbotUnauthorizedException.java"
do
echo "== $url =="
curl -fsSL "$url" | sed -n '1,240p'
echo
doneRepository: testingbot/TestingBot-Jenkins-Plugin
Length of output: 10836
Separate 401 auth failures from other TestingBot errors. TestingbotApiException covers non-401 API errors and transport/parse failures, so this catch block currently labels rate limits, server errors, and malformed responses as “authentication failed.” Catch TestingbotUnauthorizedException separately, and return a generic “TestingBot request failed” message for TestingbotApiException without echoing the raw response body.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/testingbot/TestingBotCredentials.java` around lines 339 - 342,
Update the exception handling in the credentials validation flow around
TestingbotUnauthorizedException and TestingbotApiException: keep unauthorized
failures returning the authentication-failed message, but catch
TestingbotApiException separately and return a generic “TestingBot request
failed” validation message without including e.getMessage() or raw response
details.
Source: MCP tools
1 participant
What
Adds a Test Connection button to the TestingBot credentials form — the credential verification Sauce/BrowserStack have and we lacked.
Clicking it calls the TestingBot REST API (
getUserInfo) with the entered key/secret and reports the result inline:Connection successful — signed in as <email> (<plan> plan)How
TestingBotCredentials.DescriptorImpl.doVerifyCredentials(key, secret, context)—@POST(CSRF-safe), gated to admins or users withItem.CONFIGUREon the surrounding item. Resolves the secret viaSecret.fromString(...).getPlainText()so it works whether the form submits plaintext (new credential) or the encrypted value (editing an existing one). TheTestingbotRESTclient is closed with try-with-resources.credentials.jelly: an<f:validateButton method="verifyCredentials" with="key,secret"/>.Validation
mvn -DskipTests verifyclean; jelly well-formed. Viahpi:runscript console:ERROR: Enter both a key and a secret first.badkey123/badsecret456→ERROR: TestingBot authentication failed: This request was not authorized…— confirming the call reaches the live API and surfaces auth failures.The success branch is the same code path (valid creds →
getUserInfo()returns the user); it couldn't be exercised here without real credentials, but the request/response handling is proven by the error path.Summary by CodeRabbit