URL-test: clear stale error on success; ignore cancelled-test failures

[hawkff]

Summary

Two small URL-test UX fixes addressing stale/false test results.

Changes (ConfigurationFragment.kt)

1. Clear profile.error on success. The URL-test and TCP-ping success paths set
   status=1/ping but never cleared a previously-set error, so a profile that failed once and
   later passes kept showing the old failure message until the user manually ran "Clear test
   results". Now success clears it.
2. Don't record cancelled tests as failures. The urlTest result handlers recorded
   status=2/3 + error even when the exception was just job cancellation (cancelling the test
   dialog / teardown kills the sidecar mid-handshake → IOException). Guarded with if (!isActive) break before recording, matching the existing pingTest path.

Testing

• CodeRabbit CLI: No findings.
• Verified on Android: after a re-test, profiles that pass no longer carry stale errors — the
  list shows current latency (Mieru 189ms, AmneziaWG 283/292ms, Hysteria2 197ms) or the real
  current failure reason only; no leftover stale "Get https..." strings.

Notes

• Scoped per discussion to the surgical fixes only — no "block service start while runningTest"
  guard (the test/service use separate sing-box instances; the !isActive guard fixes the actual
  false-failure symptom without the intrusive UX of blocking connects).
• Independent of PR URL-test: await external plugin loopback readiness (#46) #46 (URL-test readiness); they compose cleanly.

Greptile Summary

This PR makes two targeted UX fixes to the URL-test feature in ConfigurationFragment.kt: clearing stale profile.error when a test passes, and skipping failure recording when a test is cancelled mid-flight (so a killed-sidecar IOException doesn't get written as a profile failure).

• Clear error on success: both TCP-ping and URL-test success paths now set profile.error = null, preventing old failure messages from persisting after a profile passes a subsequent test.
• Guard cancelled-test failures: if (!isActive) break is added in the urlTest catch blocks before recording any failure state, mirroring the pre-existing pattern in pingTest. A final if (!isActive) break also guards the test.update call so cancelled results are never added to the results queue.

Confidence Score: 5/5

Safe to merge — both changes are surgical, well-scoped, and match patterns already established in the TCP-ping path.

The profile.error = null additions on success paths are straightforward and cannot regress. The !isActive guards in urlTest directly mirror the existing pingTest logic, and test.update is already a no-op for results when dialogStatus is 2, so the guards add defence-in-depth without changing normal-test behaviour. No new state, no schema changes, no new async paths introduced.

No files require special attention.

Important Files Changed

Filename	Overview
app/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.kt	Two targeted fixes: clears profile.error on success for both TCP-ping and URL-test paths, and adds isActive cancellation guards in the urlTest catch blocks, matching the pre-existing pattern in pingTest.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[urlTest coroutine - while isActive] --> B[poll next profile]
    B -->|null - queue empty| Z[break - done]
    B -->|profile| C[profile.status = 0]
    C --> D[urlTest.doTest]
    D -->|success| E[status=1, ping=result, error=null NEW]
    D -->|PluginNotFoundException| F{isActive? NEW}
    D -->|IOException / Exception| G{isActive? NEW}
    F -->|false - cancelled| Z
    F -->|true| H[status=2, error=message]
    G -->|false - cancelled| Z
    G -->|true| I[status=3, error=message]
    E --> J{isActive? NEW}
    H --> J
    I --> J
    J -->|false| Z
    J -->|true| K[test.update - add to results]
    K --> A

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[urlTest coroutine - while isActive] --> B[poll next profile]
    B -->|null - queue empty| Z[break - done]
    B -->|profile| C[profile.status = 0]
    C --> D[urlTest.doTest]
    D -->|success| E[status=1, ping=result, error=null NEW]
    D -->|PluginNotFoundException| F{isActive? NEW}
    D -->|IOException / Exception| G{isActive? NEW}
    F -->|false - cancelled| Z
    F -->|true| H[status=2, error=message]
    G -->|false - cancelled| Z
    G -->|true| I[status=3, error=message]
    E --> J{isActive? NEW}
    H --> J
    I --> J
    J -->|false| Z
    J -->|true| K[test.update - add to results]
    K --> A

Loading

_{Reviews (1): Last reviewed commit: "fix(urltest): clear stale error on succe..." | Re-trigger Greptile}

[coderabbitai]

📝 Walkthrough

Walkthrough

In ConfigurationFragment, two connection test methods are patched: pingTest now clears profile.error before updating state on TCP success, and urlTest clears profile.error on success and adds !isActive checks in exception handlers to skip recording errors caused by test cancellation or teardown.

Changes

Connection Test Error Handling

Layer / File(s)	Summary
Clear stale errors and guard cancelled-test failures app/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.kt	pingTest clears profile.error on TCP success before calling test.update(profile). urlTest clears profile.error on success (status = 1), and both PluginNotFoundException and generic Exception catch blocks now early-break without updating the profile when !isActive; an additional !isActive guard is added before the profile update call.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐇 A test once failed, its ghost lingered on,
Stale error messages refused to be gone.
But now on success the error is cleared,
And cancelled mid-hop? No record is smeared.
The profile stays clean, the rabbit hops free!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main changes: clearing stale errors on successful URL tests and ignoring cancelled test failures, which matches the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description clearly and accurately describes the changes made to ConfigurationFragment.kt: clearing stale errors on success and guarding against recording cancelled tests as failures.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}