Skip to content

split tunneling: treat FFI "ok" response as success, not error#8691

Merged
myleshorton merged 2 commits intogarmr/radiance-daemon-refactorfrom
fisk/fix-split-tunnel-ok-success
Apr 23, 2026
Merged

split tunneling: treat FFI "ok" response as success, not error#8691
myleshorton merged 2 commits intogarmr/radiance-daemon-refactorfrom
fisk/fix-split-tunnel-ok-success

Conversation

@myleshorton
Copy link
Copy Markdown
Contributor

Summary

  • _runSplitTunnelCall was checking result != nullptr and treating any non-null FFI return as an error. But the Go side returns C.CString("ok") on success (a non-null C string), so every successful add/remove was reported to the UI as a failure with the message "ok".
  • Fix: mirror the checkAPIError convention — treat literal "ok" as success, parse JSON {"error": "..."} bodies for the error message, and fall back to the raw string otherwise.

User-visible effect

Reported in getlantern/engineering#3291 against Windows 9.0.29 build 481 (Freshdesk #173656):

  • Adding a website in split tunneling showed an unstyled default snackbar reading "OK" (Material's default SnackBar rendering failure.localizedErrorMessage).
  • The website appeared to not be saved — but it actually was. The provider's reloaded flag was never set, so the on-screen list never re-fetched.
  • Re-clicking "Add" with the same domain created a duplicate entry on disk (visible as repeated items in the user's split-tunnel.json) because the provider's local "already-added" check worked against a stale copy.

One root-cause fix addresses all three symptoms.

Test plan

  • Add a website in split tunneling → no snackbar, website appears in "websites bypassing the VPN" list immediately
  • Add the same website again → handled gracefully (provider short-circuits, no duplicate on disk)
  • Remove a website → list refreshes, no spurious snackbar
  • Trigger a real error (e.g. malformed input that makes it past validation) → error message surfaces correctly (no longer hidden behind "OK")

🤖 Generated with Claude Code

_runSplitTunnelCall was checking `result != nullptr` and treating any
non-null return as an error message. But the Go FFI
(lantern-core/ffi/ffi.go) returns C.CString("ok") on success for both
addSplitTunnelItem and removeSplitTunnelItem — a non-null C string.

As a result, every successful add/remove was being reported to the UI as
a failure with message "ok". Symptoms:

- Adding a website in split tunneling showed an unstyled default
  snackbar reading "OK" (the default Material SnackBar rendering
  failure.localizedErrorMessage).
- The website appeared to not be saved — but it actually was; the
  provider's `reloaded` flag was never set, so the on-screen list never
  re-fetched from the backend.
- Re-clicking "Add" with the same domain created a duplicate entry on
  disk (visible as repeated items in split-tunnel.json) because the
  provider's local "already-added" check worked against a stale copy
  that had never been refreshed.

Fix: mirror the checkAPIError convention — treat literal "ok" as
success, parse JSON {"error": "..."} bodies for the error message, and
fall back to the raw string otherwise.

Reported in getlantern/engineering#3291 against Windows 9.0.29 build 481
(Freshdesk #173656).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 23, 2026 13:11
Rather than hardcoding 'ok', use the existing _ffiOkResults set
({'ok', 'true'}) defined at the top of this file so the split-tunnel
path stays in sync with the other FFI success checks (e.g.
_setupRadiance at line 201).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes split-tunneling add/remove calls incorrectly treating successful Go FFI responses ("ok") as errors, which caused spurious “OK” snackbars and stale UI state.

Changes:

  • Update _runSplitTunnelCall to treat nullptr and literal "ok" as success.
  • Parse JSON error bodies ({"error":"..."}) from Go FFI and surface the decoded error message instead of the raw JSON.
  • Ensure non-null returned C strings are freed after decoding.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@myleshorton myleshorton merged commit f41a809 into garmr/radiance-daemon-refactor Apr 23, 2026
@myleshorton myleshorton deleted the fisk/fix-split-tunnel-ok-success branch April 23, 2026 14:38
myleshorton added a commit that referenced this pull request Apr 23, 2026
The local showSnackbar helper in website_domain_input was using
Material's default ScaffoldMessenger.showSnackBar(SnackBar(content:
Text(message))) — producing an unstyled grey/dark snackbar that the rest
of the app doesn't use. Every call site in this file is an error path
(empty input, invalid domain, already-added, backend failure), so route
them through context.showSnackBarError which applies the app's rounded,
floating, red-background error style.

Follow-up to #8691. Addresses the "unstyled snackbar" symptom in
getlantern/engineering#3291 issue 3 for any remaining error surface
after the FFI "ok" fix.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

2 participants