Skip to content

fix: always reply to track method channel calls and surface failures - #75

Open
vahidlazio wants to merge 2 commits into
mainfrom
vahidt/fix-track-method-channel-reply
Open

fix: always reply to track method channel calls and surface failures#75
vahidlazio wants to merge 2 commits into
mainfrom
vahidt/fix-track-method-channel-reply

Conversation

@vahidlazio

@vahidlazio vahidlazio commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

track was the only method channel case that never sent a reply. Every other case calls result(...) / result.success(...), so a track call left the Dart-side reply pending forever — on both iOS and Android. iOS additionally swallowed every failure via try?.

Defects fixed

Defect Platform
track never replies on the method channel, leaving the Dart future pending iOS + Android
All track errors silently discarded by try? iOS

Changes

  • iOS (ConfidenceFlutterSdkPlugin.swift) — replaced try? with do/catch: reply result("") on success, FlutterError(code: "TRACK_FAILED", ...) on failure, and log via NSLog to match the other catch blocks in the file. The argument-guard path now also replies, consistent with neighbouring cases.
  • Android (ConfidenceFlutterSdkPlugin.kt) — reply result.success(null), and translate a thrown exception into result.error("TRACK_FAILED", ...) instead of letting it escape the channel handler.
  • Dart (confidence_flutter_sdk_method_channel.dart) — track() deliberately stays void; changing it to return a Future would be a breaking public API change. Because the returned future is therefore never awaited, a native error reply would surface as an unhandled async error in the host app, so a catchError handler now logs it instead.

Tests

Two tests added to the existing method channel suite:

  • track forwards the event name and typed data
  • track does not raise an unhandled async error when native fails — this one fails without the Dart catchError guard (verified by reverting the guard), so it genuinely pins the regression

Verification

flutter analyze clean; all 4 method channel tests pass.

Verified end-to-end on both platforms against a real Confidence account, driving the example app:

iOS (iPhone 15 simulator) — explicit upload status:

[Emitting event] <event>
[Event flushed]
Event upload: HTTP status 200. Events: <event>

Android (Pixel 7 Pro, API 33):

[EmitEvent ] EngineEvent(eventDefinition=<event>, ...)
[DiskWrite ] EngineEvent(eventDefinition=<event>, ...)
Flush policy ManualFlushPolicy triggered to flush. Flushing.
Uploading events

with no error logged, and the on-disk batch file absent afterwards — the SDK only deletes a batch after a successful upload, retaining it for retry on failure.

flush has the same defect — now fixed here too

A follow-up review found the earlier note was incomplete: "flush" had the identical
missing-reply defect on both platforms, not Android only.

  • Android (ConfidenceFlutterSdkPlugin.kt) never replied at all.
  • iOS (ConfidenceFlutterSdkPlugin.swift) replied in the guard branch but fell through
    to break; after confidence.flush() with no reply.

Both are fixed in this PR. Confidence.flush() is non-throwing on iOS, so no do/catch
was added there (an unreachable catch would only produce a warning); Android wraps the
call and replies success or FLUSH_FAILED, matching track.

Because the platform interface types flush() as void, callers discard the future — so
now that Android can reply with an error, the Dart side swallows and logs it, as track
already does.

Still out of scope (pre-existing, filed separately)

  • fetchAndActivate, activateAndFetchAsync and readAllFlags reply from inside
    coroutineScope.launch {} on Dispatchers.IO (iOS: Task {}). If the suspending call
    throws, the reply never happens and the Dart future never completes; replies should also
    be marshalled to the platform thread.
  • Dart toTypedValue maps unsupported values (e.g. DateTime, null) to
    {'type': 'unknown'}, and iOS convertValue's default: returns
    ConfidenceValue(integer: 0) — so those publish as 0 rather than being rejected.

🤖 Generated with Claude Code

vahidlazio and others added 2 commits September 7, 2026 13:15
`track` was the only method channel case that never replied. Every other
case calls result(...)/result.success(...), so a track call left the
Dart-side reply pending forever — on both iOS and Android. iOS also
swallowed every failure via `try?`.

- iOS: replace `try?` with do/catch, reply result("") on success and a
  FlutterError(TRACK_FAILED) on failure, and log via NSLog like the other
  catch blocks. Also reply on the argument-guard path, matching neighbours.
- Android: reply result.success(null), and translate a thrown exception
  into result.error(TRACK_FAILED) rather than letting it escape the
  channel handler.
- Dart: track() stays `void` (making it a Future would break the public
  API), and the unawaited future now has a catchError so a native error
  reply is logged instead of becoming an unhandled async error.

Adds two method channel tests; the error one fails without the Dart guard.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
flush had the same dangling-reply defect as track, on BOTH platforms — not
Android only as previously noted. iOS replied in the guard branch but fell
through to `break;` after `confidence.flush()` with no reply, and Android
never replied at all. Either way the Dart future never completed.

iOS now replies after flushing. Confidence.flush() is non-throwing there, so
no do/catch is added — an unreachable catch would only warn. Android wraps
the call and replies success or FLUSH_FAILED, matching the shape track uses.

The platform interface types flush() as void, so callers discard the future.
Now that Android can reply with an error, that rejection would surface as an
unhandled async error in the host app, so the Dart side swallows and logs it
as track already does.

Co-Authored-By: Claude Opus 4.6 (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.

1 participant