Repository navigation
fix(cloud_functions): surface httpsCallable stream errors as FirebaseFunctionsException - #18834
Conversation
…FunctionsException Android: StreamResponseSubscriber.onError dropped the throwable and only ended the stream, so a failing streaming callable completed with no result and no error. It now sends the error with the code, message and details mapping callable errors use, then ends the stream. Dart: errors emitted by the event channel are forwarded by `yield*` instead of being thrown into the surrounding `catch`, so they reached apps as raw PlatformExceptions. They are now converted on the stream.
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
…ix/functions-stream-errors
…nd macOS iOS/macOS: stream errors were always reported with the code `unknown`. They now use the same code, message and details mapping as non-streaming calls (for example `invalid-argument` for an `HttpsError`). The stream also ends natively after an error, so errored streams are no longer left open (a small leak). Dart: a stream error with no message, in both the PlatformException and its details, no longer surfaces as a TypeError; it becomes a FirebaseFunctionsException with an empty message. Android: a stream that fails before subscribing now sends the error details and ends the stream, like errors from the function itself, and cancelling such a stream no longer throws a NullPointerException. Known limitation: on Android the Firebase SDK reports every stream error as INTERNAL, with the function's code only inside the message (PublisherStream.processEvent), so Android apps see `code: 'internal'` for stream errors until the SDK is fixed. cloud_functions still allows cloud_functions_platform_interface ^6.0.7. With an older platform interface, these errors arrive as a raw PlatformException rather than a FirebaseFunctionsException.
| eventSink.error( | ||
| "firebase_functions", t.message, FlutterFirebaseFunctionsPlugin.getExceptionDetails(t)) | ||
| eventSink.endOfStream() | ||
| } |
There was a problem hiding this comment.
Seems android can report the function's real code. PublisherStream sets the code to INTERNAL, but details is the decoded error {status, message, details}, and the exception message is that map's toString(). In StreamResponseSubscriber.onError, parse it the way FirebaseFunctionsException.fromResponse does: status becomes the code, message becomes the message, and only the inner details is kept. Do this on the stream path only. An unknown status stays internal.
There was a problem hiding this comment.
Done in c10dc15: on the stream path, onError now reads the decoded error the way fromResponse does. status becomes the code, message the message, and only the inner details is kept. As in fromResponse, an unknown status gives internal without message or details. The e2e now expects invalid-argument on Android too and passes on the emulator.
While verifying it I found a crash on this branch, fixed in ac79230. When reading the stream fails after the response started (timeout, dropped connection, bad JSON), PublisherStream passes the error as details, and the codec threw Unsupported value on the main thread. It now maps to deadline-exceeded/unavailable like calls. Details are in the PR description.
…n sent for stream errors
The Android SDK reports an error sent by the function during a stream as
INTERNAL, with the decoded error ({status, message, details}) as details and
its toString() as message. The plugin now reads the code, message and details
from it, as FirebaseFunctionsException.fromResponse does for calls, so stream
errors on Android match iOS, macOS and web (e.g. invalid-argument rather than
internal). Like fromResponse, an unknown status makes the error internal,
without message or details.
…eading When reading a stream fails (a timeout or a dropped connection after the response started, or invalid JSON), PublisherStream passes the error as the exception's details rather than its cause. The plugin sent it as additionalData, which the event channel codec can't encode, so the app crashed on the main thread. The error is now used as the cause, so a timeout maps to deadline-exceeded and an IO error to unavailable, as for calls, and is never sent as details. The test event sink now encodes events like EventChannel, so a value the codec can't send fails the test.
Description
Errors from
httpsCallable(...).stream()did not reach apps asFirebaseFunctionsExceptionwith the right code. There were three bugs.1. Android: an error looked like a normal end of stream.
StreamResponseSubscriber.onErrordropped the throwable and only calledendOfStream(). So a streaming callable that failed (unauthenticated, network error, function threw) completed with no result and no error in Dart. It now sendseventSink.error(...)on the main thread, then ends the stream. The error uses the same code, message andadditionalDatamapping as non-streaming calls.getExceptionDetailsmoved to the companion object so the subscriber can share it. It now also accepts an unwrappedFirebaseFunctionsException: the stream publisher passes the exception as is, whileTasks.awaitwraps callable errors in anExecutionException.The SDK reports an error the function sent during a stream as
INTERNAL, with the decoded error ({status, message, details}) as details and itstoString()as message (PublisherStream.processEvent).StreamResponseSubscriber.onErrorreads it asFirebaseFunctionsException.fromResponsedoes for calls:statusbecomes the code,messagethe message, and only the innerdetailsis kept. As infromResponse, an unknown status makes the errorinternal, without message or details. This only runs on the stream path.2. Dart: stream errors skipped the conversion. In
MethodChannelHttpsCallable.stream,yield*adds the event channel's errors to the output stream. It does not throw them into the surroundingtry, soconvertPlatformExceptionnever ran. Apps got a rawPlatformExceptioninstead of aFirebaseFunctionsException; this is what iOS and macOS apps saw. Errors are now converted on the stream withhandleError(convertPlatformException).Reproduced on an Android emulator against the functions emulator. With
testFunctionDefaultRegion,callable.stream({})makes the function throwHttpsError('invalid-argument'):FirebaseFunctionsException(invalid-argument), then donePlatformException(unknown), stream left openFirebaseFunctionsException(invalid-argument), then done3. iOS/macOS: every stream error had the code
unknown.FunctionsStreamHandlerignored the SDK'sFunctionsErrorcode. It now uses the plugin'screateFlutterError(from:), so stream errors get the same code, message and details as non-streaming calls. It also sendsFlutterEndOfEventStreamafter every error, as Android does; before, errored streams were never closed natively (a small leak).Review fixes:
PlatformException.messagenordetails['message']) mademessage!throw insidehandleError, so the stream emitted aTypeError. The conversion now falls back to an empty message, like_flutterfire_internals.FirebaseFunctionsStreamHandler's catch) now sends the error details and ends the stream, likeonError.onCancelno longer throws aNullPointerExceptionwhen setup failed and there is no subscriber.FirebaseFunctionsExceptiondoc no longer says stream errors areunknownon Apple platforms and silent on Android.internal.PublisherStream.processSSEStream/processEventpass the error as the exception's details, not its cause. The plugin sent thatThrowableasadditionalData, the event channel codec threwIllegalArgumentException: Unsupported valueon the main thread, and the app died. Reproduced on the Android emulator: a 3 s callable timeout ontestFunctionTimeoutwith its chunk at 2.9 s givesFATAL EXCEPTION: main … Unsupported value: 'java.io.InterruptedIOException: timeout'.getExceptionDetailsnow uses such an error as the cause (so a timeout maps todeadline-exceededand an IO error tounavailable, as for calls) and never sends it as details. With the fix, the same run emitsdeadline-exceeded. The probe is too timing-dependent for CI, so unit tests cover it.Known limit:
cloud_functionsstill allowscloud_functions_platform_interface: ^6.0.7. With the new native code and an older platform interface, stream errors arrive as a rawPlatformException, which still beats a stream that ends silently.Tests
cloud_functions_platform_interface).method_channel_https_callable_test.dartmocks the stream's event channel. A native error with details arrives as aFirebaseFunctionsExceptionwith the right code, message and details, after earlier data events. An error without details arrives with codeunknown. Both fail onmain, where they receive aPlatformException.FirebaseFunctionsExceptionwith an empty message. This test, and one inexception_test.dart, fail without the null fix ("Null check operator used on a null value").android-unit-tests).StreamResponseSubscriberTest:onErrorsends the functions error, then ends the stream. AnIOExceptioncause maps tounavailable, as it does for callable errors, and any other throwable tounknown.onCompleteonly ends the stream. The subscriber now takes anExecutor(by default the main-threadHandler), so the tests run without a Looper. TheonErrortests fail without the fix, since onlyendOfStreamis recorded.onError_reportsTheCodeAndMessageTheFunctionSent(invalid-argument, the function's message and inner details),onError_reportsAnUnknownStatusAsInternal,onError_keepsTheMessageWhenTheFunctionSentNoStatusandonError_usesTheCodeNameWhenTheFunctionSentNoMessage.onError_mapsStreamReadFailuresLikeNetworkErrors(unavailable),onError_mapsStreamReadTimeoutsToDeadlineExceededandonError_doesNotSendTheErrorBehindInvalidJson.RecordingEventSinknow encodes each event withStandardMethodCodec, asEventChanneldoes, so a value the codec can't send fails the test. All three fail on the previous code withUnsupported value.FirebaseFunctionsStreamHandlerTest: a setup failure sends the error with details, then ends the stream, and a lateronCanceldoesn't throw. Both fail on the previous code (detailsnullwith no end of stream; NPE). The test builds a realFirebaseFunctionsthat never reaches the network.FlutterFirebaseFunctionsPluginTestgains a test for the wrapped callable path.TestFunctionsExceptions.javaandTestFirebaseFunctions.javaexist because those constructors are Kotlininternal.e2e_test.dart,e2e-functions):emits a [FirebaseFunctionsException] on errorstreams fromtestFunctionDefaultRegionwith invalid input. It expects aFirebaseFunctionsExceptionwhose message contains the server message, then the end of the stream. The code must beinvalid-argumenton every platform.internalbefore the review fix) and on iOS ([firebase_functions/unknown] Invalid test requested.).invalid-argument. Web was not run locally.Related Issues
None found. #18824 also edits the functions
e2e_test.dart;mainis merged in.Checklist
///).melos run analyze) does not report any problems on my PR.Breaking Change
Streams that used to end silently on Android now emit an error. On iOS and macOS, stream errors change type from
PlatformExceptiontoFirebaseFunctionsException, and their code changes fromunknownto the real code. None of these was the documented behaviour.