surface network errors from integration token validation#1062
Merged
mergify[bot] merged 2 commits intomainfrom Mar 27, 2026
Merged
surface network errors from integration token validation#1062mergify[bot] merged 2 commits intomainfrom
mergify[bot] merged 2 commits intomainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUnwraps network errors from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
I tested it locally. Here's a screenshot: 
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/backend/handlers/integration_validation_test.go`:
- Around line 50-64: Tests calling ValidateGitHubToken rely on real network
behavior and only assert on the error message when an error happens, so they can
pass without exercising the network-failure path; change the tests to
deterministically simulate a connection failure (like the existing
connRefusedURL() approach used for GitLab/Jira) by injecting a failing endpoint
or by using a custom http.Client/Transport stub so ValidateGitHubToken’s network
helper code is exercised; update the test(s) to call ValidateGitHubToken (or its
underlying HTTP helper used by ValidateGitLabToken) with an unreachable URL or
mocked Transport and assert the error message begins with "request failed: " and
is not exactly "request failed".
In `@components/backend/handlers/integration_validation.go`:
- Around line 15-20: The networkError function currently does a direct type
assertion against *url.Error which misses wrapped errors; change it to use
errors.As to detect and extract a *url.Error from a possibly wrapped error (and
return urlErr.Err), update the function signature to import/use the errors
package, and keep the fallback of returning the original err when no *url.Error
is found; reference the networkError function and the *url.Error type when
making this change.
🪄 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
Run ID: 1ca76916-ff37-4ad2-a7a6-e972b64e92c8
📒 Files selected for processing (2)
components/backend/handlers/integration_validation.gocomponents/backend/handlers/integration_validation_test.go
ktdreyer
force-pushed
the
kdreyer/fix-integration-validation-errors
branch
from
27170ad to
cb62764
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/backend/handlers/integration_validation.go (1)
97-100:⚠️ Potential issue | 🟠 MajorFail fast on Jira request-construction errors.
Line 98 currently continues on
http.NewRequestWithContextfailures, so a malformed base URL falls through to the Line 134return true, nilpath and/api/auth/jira/testcan report success for an unusable URL. Since both fallback endpoints share the same base, this branch should return a sanitized error instead of retrying.💡 Proposed fix
- req, err := http.NewRequestWithContext(ctx, "GET", apiURL, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) if err != nil { - continue + return false, fmt.Errorf("failed to create request") }Please add a malformed-URL regression test alongside the new Jira coverage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/backend/handlers/integration_validation.go` around lines 97 - 100, The code currently continues on http.NewRequestWithContext errors which lets a malformed base URL incorrectly report success; update the Jira request-construction branch to return a sanitized, non-sensitive error (e.g., "invalid Jira URL") instead of continue so the handler for /api/auth/jira/test fails fast; modify the function that builds/executes the Jira request (the block calling http.NewRequestWithContext) to return that error up the stack and add a regression unit test for a malformed-URL case alongside the new Jira coverage to assert the endpoint returns failure for an invalid base URL.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@components/backend/handlers/integration_validation.go`:
- Around line 97-100: The code currently continues on http.NewRequestWithContext
errors which lets a malformed base URL incorrectly report success; update the
Jira request-construction branch to return a sanitized, non-sensitive error
(e.g., "invalid Jira URL") instead of continue so the handler for
/api/auth/jira/test fails fast; modify the function that builds/executes the
Jira request (the block calling http.NewRequestWithContext) to return that error
up the stack and add a regression unit test for a malformed-URL case alongside
the new Jira coverage to assert the endpoint returns failure for an invalid base
URL.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7ead27b1-2569-4fca-af80-7b2983b7f783
📒 Files selected for processing (2)
components/backend/handlers/integration_validation.gocomponents/backend/handlers/integration_validation_test.go
Contributor
Author
|
@Mergifyio rebase |
✅ Branch has been successfully rebased |
ktdreyer
force-pushed
the
kdreyer/fix-integration-validation-errors
branch
from
cb62764 to
b51abf5
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/backend/handlers/integration_validation.go`:
- Around line 94-95: The current logic treats any recorded network error
(lastNetErr) as a transport failure even when another probe produced a valid
HTTP response; change this by tracking whether any probe produced an HTTP
response (e.g., introduce a boolean like sawHTTPResponse or gotHTTPResponse
inside the probe loop) and only treat lastNetErr as a transport-level failure
when no probe produced an HTTP response (i.e., if !sawHTTPResponse && lastNetErr
!= nil). Update the places using lastNetErr (around the probe loop and the
checks referenced by lastNetErr at lines ~108-109 and ~128-131) to consult the
new flag instead of treating any network error as definitive.
🪄 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
Run ID: 43c70363-7b85-42cf-99cf-39ce6e3b26f5
📒 Files selected for processing (2)
components/backend/handlers/integration_validation.gocomponents/backend/handlers/integration_validation_test.go
ktdreyer
force-pushed
the
kdreyer/fix-integration-validation-errors
branch
from
ec8356c to
18d510e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/backend/handlers/integration_validation.go`:
- Around line 93-95: The validation currently treats malformed Jira URLs as
success because request-construction failures leave lastNetErr nil and fall
through to returning true; update the probe logic in integration_validation.go
so that any error returned when building an *http.Request* (capture errors where
request creation fails) is assigned to lastNetErr and does not set
sawHTTPResponse, and after the probe loop if sawHTTPResponse is false return
false, lastNetErr (or a wrapped error) instead of true,nil; apply the same
change to the other probe blocks mentioned (where got401, lastNetErr,
sawHTTPResponse are used) and add a regression test that passes an obviously
malformed Jira URL and asserts validation returns false with the construction
error.
🪄 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
Run ID: 726f8f1a-f719-4504-8364-b6ad5ab3b446
📒 Files selected for processing (2)
components/backend/handlers/integration_validation.gocomponents/backend/handlers/integration_validation_test.go
Contributor
Review Queue Status
Action needed: Address CodeRabbit Major issues on error handling logic
|
The "request failed" message hid the root cause (TLS, DNS, timeout) because the code avoided wrapping *url.Error to prevent leaking tokens. Unwrap *url.Error to extract only the inner network error, which is safe to expose. Users connecting to internal instances (eg GitLab behind a corporate CA) now see actionable diagnostics like "request failed: tls: failed to verify certificate" instead of the opaque "request failed". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously, if one API version probe (e.g. v3) received a normal HTTP response (like 404) but the other (v2) hit a network error, the function returned "request failed" even though the server was reachable. Track whether any probe received an HTTP response and only surface the network error when none did. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ktdreyer
force-pushed
the
kdreyer/fix-integration-validation-errors
branch
from
18d510e to
22c072a
Compare
bobbravo2
approved these changes
Mar 27, 2026
Member
bobbravo2
left a comment
bobbravo2
left a comment
There was a problem hiding this comment.
Lgtm. Nice work with the code rabbit, and thanks for tracking follow up work!
Merge Queue Status
This pull request spent 11 seconds in the queue, including 1 second running CI. Required conditions to merge
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
*url.ErrorinValidateGitHubToken,ValidateGitLabToken,ValidateGoogleToken, andValidateJiraTokento surface the inner network error (TLS, DNS, timeout, connection refused) without leaking the request URL or token.Fixes #1037
Test plan
go vet ./handlers/passesgo test -tags test ./handlers/ -ginkgo.focus="Integration Validation")make kind-up— error now shows TLS/certificate cause