Skip to content

fix multipart upload crash ClientException Bad file descriptor#7507

Open
krushnarout wants to merge 1 commit into
mainfrom
fix/multipart-bad-file-descriptor
Open

fix multipart upload crash ClientException Bad file descriptor#7507
krushnarout wants to merge 1 commit into
mainfrom
fix/multipart-bad-file-descriptor

Conversation

@krushnarout
Copy link
Copy Markdown
Member

Crash

(null).makeMultipartApiCall

Error: FlutterError - ClientException: Bad file descriptor, uri=https://api.omi.me/v1/sync-local-files

Crashlytics: https://console.firebase.google.com/u/0/project/based-hardware/crashlytics/app/ios:com.friend-app-with-wearable.ios12/issues/780a8bb89b21adf52eabdac3034cd749?time=7d&types=crash&sessionEventKey=6453a786b0ce4ba0a4c6c96707e1aae3_2222621226552694960

Logs:

Fatal Exception: FlutterError
0  ???  0x0 Future.timeout.<fn> (dart:async)
1  ???  0x0 (null).makeMultipartApiCall + 271 (shared.dart:271)
2  ???  0x0 (null).syncLocalFilesV2 + 304 (conversations.dart:304)
3  ???  0x0 LocalWalSyncImpl.syncAll + 619 (local_wal_sync.dart:619)
4  ???  0x0 SyncProvider._performSync + 271 (sync_provider.dart:271)
5  ???  0x0 SyncProvider._autoUploadPendingPhoneFiles + 175 (sync_provider.dart:175)

Fix

Two changes in shared.dart:

  1. Added Bad file descriptor to _isTransientNetworkError — when a multipart upload times out, the HTTP layer closes the socket and subsequent reads on the in-flight file stream raise EBADF wrapped in a ClientException. This is an OS-level transient error, not a code bug, so it was being incorrectly reported to Crashlytics. Adding it to the transient list stops the false crash report; the error still rethrows and is caught gracefully by the WAL sync retry loop.

  2. Fixed _sendMultipartWithProgress stream subscription lifecycle — the progress-stream subscription was fire-and-forget with no error handler that closed the sink. On timeout or error, the orphaned subscription would continue pumping data into a closed sink, causing unhandled zone errors. Now the subscription is stored, cancelOnError: true stops it on first error, the onError handler closes the sink immediately, and future.whenComplete(subscription.cancel) cleans it up when the send finishes.

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 26, 2026

Greptile Summary

This PR fixes a Crashlytics-reported crash during multipart uploads in shared.dart by (1) classifying Bad file descriptor as a transient network error so it is no longer forwarded to Crashlytics, and (2) properly managing the progressStream subscription lifecycle so that orphaned subscriptions can no longer pump data into a closed HTTP sink after a timeout.

  • Transient-error classifier (_isTransientNetworkError): adds Bad file descriptor alongside other OS-level transport errors so the WAL retry loop handles the error gracefully rather than logging a false crash.
  • Subscription lifecycle (_sendMultipartWithProgress): the subscription is now stored, cancelOnError: true halts it on the first stream error, the onError handler closes the sink immediately, and future.whenComplete(subscription.cancel) tears down the subscription when the HTTP future resolves — preventing the orphaned-subscription zone errors described in the issue.

Confidence Score: 4/5

The fix is a net improvement that eliminates the reported crash; the one remaining edge case is a theoretical secondary zone error in a rare ordering of timeout + EBADF, which is easy to harden with a try/catch.

Both changes target real, well-characterised failure modes. The subscription lifecycle fix correctly cancels the orphaned listener on all three exit paths (normal, timeout, stream error). The one concern is the onError handler calling sink.addError after the HTTP consumer may have already detached from the sink, which could produce a new unhandled zone error in the same category as the original crash — but only in a narrow race window and easily guarded.

Only app/lib/backend/http/shared.dart changed; specifically the onError lambda in _sendMultipartWithProgress warrants a closer look for the sink-after-timeout edge case.

Important Files Changed

Filename Overview
app/lib/backend/http/shared.dart Two targeted fixes: adds 'Bad file descriptor' to transient-error classifier and overhauls subscription lifecycle in _sendMultipartWithProgress with cancelOnError, explicit sink close in onError, and whenComplete cleanup. One edge case remains where sink.addError in the onError handler may produce a secondary zone error if the HTTP future already completed.

Sequence Diagram

sequenceDiagram
    participant Caller as makeMultipartApiCall
    participant Send as _sendMultipartWithProgress
    participant Sub as progressStream.listen
    participant Sink as streamedRequest.sink
    participant Pool as HttpPoolManager

    Caller->>Send: call with onProgress
    Send->>Sub: subscribe (cancelOnError: true)
    Send->>Pool: sendStreaming(streamedRequest)
    Pool-->>Send: future

    note over Send: Stores subscription<br/>Registers whenComplete(cancel)

    alt Normal completion
        Sub->>Sink: sink.add(data) [per chunk]
        Sub->>Sink: onDone → sink.close()
        Pool-->>Caller: StreamedResponse
        Send->>Sub: whenComplete → subscription.cancel()
    else Timeout / HTTP error
        Pool-->>Caller: throws TimeoutException
        Send->>Sub: whenComplete → subscription.cancel()
        note over Sub: subscription stopped,<br/>no more data pumped
    else Stream error (e.g. EBADF)
        Sub->>Sink: onError → sink.addError + sink.close()
        note over Sub: cancelOnError cancels subscription
        Pool-->>Caller: throws error
    end

    Caller->>Caller: catch → _isTransientNetworkError check
    note over Caller: "Bad file descriptor" now<br/>classified as transient → no Crashlytics report
Loading

Reviews (1): Last reviewed commit: "fix multipart upload crash ClientExcepti..." | Re-trigger Greptile

Comment on lines +242 to +245
onError: (Object e, StackTrace st) {
streamedRequest.sink.addError(e, st);
streamedRequest.sink.close();
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Guard the sink operations with a try/catch so that a StateError ("Cannot add event after closing") thrown when the HTTP consumer has already finished does not escape as an unhandled zone error. When the HTTP future times out and completes before the source stream emits EBADF, the HTTP consumer has already disconnected from streamedRequest.sink. If the file stream subsequently raises an error, onError calls sink.addError(e, st) on a sink whose underlying StreamController may have no listeners — the Dart runtime would then re-raise the error as an unhandled zone error, the same class of crash the PR is fixing.

Suggested change
onError: (Object e, StackTrace st) {
streamedRequest.sink.addError(e, st);
streamedRequest.sink.close();
},
onError: (Object e, StackTrace st) {
try {
streamedRequest.sink.addError(e, st);
streamedRequest.sink.close();
} catch (_) {
// Sink already closed by the HTTP layer; ignore.
}
},

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