fix(functions): use notifyError() instead of throwing #6773

thatfiredev · 2025-03-14T13:25:50Z

Throwing inside the PublisherStream causes a Runtime exception that can't be caught in the call site.
Instead, we should use notifyError() so that the error can be caught in the Subscriber#onError() function.

I tried to catch the exception using both of the code snippets below, but none of them worked. (they both work with the changes in this PR):

functions.getHttpsCallable("nonExistentFunction")
    .stream().asFlow()
    .catch {
        // Handle error for a 404 function
    }
    .collect {
        // ...
    }

try {
    functions.getHttpsCallable("nonExistentFunction")
        .stream().asFlow()
        .collect {
            // ...
        }
} catch(e: Exception) {
    // Handle error for a 404 function
}

Runtime exception thrown:

FATAL EXCEPTION: OkHttp Dispatcher
    Process: com.google.samples.quickstart.functions, PID: 13321
    com.google.firebase.functions.FirebaseFunctionsException: Value <html><head> of type java.lang.String cannot be converted to JSONObject Unexpected Response:
                                                                             
    <html><head>
    <meta http-equiv="content-type" content="text/html;charset=utf-8">
    <title>404 Page not found</title>
    </head>
        <body text=#000000 bgcolor=#ffffff>
            <h1>Error: Page not found</h1>
            <h2>The requested URL was not found on this server.</h2>
            <h2></h2>
        </body></html>
                                                                              
    at com.google.firebase.functions.PublisherStream.validateResponse(PublisherStream.kt:316)
    at com.google.firebase.functions.PublisherStream.access$validateResponse(PublisherStream.kt:41)
    at com.google.firebase.functions.PublisherStream$startStreaming$1$4.onResponse(PublisherStream.kt:161)
    at okhttp3.RealCall$AsyncCall.execute(RealCall.java:203)
    at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1156)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:651)
    at java.lang.Thread.run(Thread.java:1119)

github-actions · 2025-03-14T13:26:06Z

📝 PRs merging into main branch

Our main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released.

google-oss-bot · 2025-03-14T13:26:33Z

	1 Warning
⚠️	Did you forget to add a changelog entry? (Add the 'no-changelog' label to the PR to silence this warning.)

Generated by 🚫 Danger

google-oss-bot · 2025-03-14T13:29:43Z

Coverage Report ¹

Affected Products

firebase-functions
Overall coverage changed from 24.90% (b9be9f1) to 24.83% (f63fbbc) by -0.07%.

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/fxPPDRtMia.html

github-actions · 2025-03-14T13:30:23Z

Test Results

14 files ±0 14 suites ±0 13s ⏱️ -1s
23 tests ±0 23 ✅ ±0 0 💤 ±0 0 ❌ ±0
46 runs ±0 46 ✅ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 6b62413. ± Comparison against base commit b9be9f1.

♻️ This comment has been updated with latest results.

google-oss-bot · 2025-03-14T13:35:45Z

Size Report ¹

Affected Products

firebase-functions
Type Base (b9be9f1) Merge (f63fbbc) Diff
aar 80.4 kB 80.5 kB +10 B (+0.0%)
apk (release) 5.46 MB 5.46 MB +36 B (+0.0%)

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/ain2POUgsf.html

Throwing inside the `PublisherStream` causes a Runtime exception that can't be caught in the call site. Instead, we should use `notifyError()` so that the error can be caught in the `Subscriber#onError()` function. I tried to catch the exception using both of the code snippets below, but none of them worked. (they both work with the changes in this PR): ```kotlin functions.getHttpsCallable("nonExistentFunction") .stream().asFlow() .catch { // Handle error for a 404 function } .collect { // ... } ``` ```kotlin try { functions.getHttpsCallable("nonExistentFunction") .stream().asFlow() .collect { // ... } } catch(e: Exception) { // Handle error for a 404 function } ``` Runtime exception thrown: ``` FATAL EXCEPTION: OkHttp Dispatcher Process: com.google.samples.quickstart.functions, PID: 13321 com.google.firebase.functions.FirebaseFunctionsException: Value <html><head> of type java.lang.String cannot be converted to JSONObject Unexpected Response: <html><head> <meta http-equiv="content-type" content="text/html;charset=utf-8"> <title>404 Page not found</title> </head> <body text=#000000 bgcolor=#ffffff> <h1>Error: Page not found</h1> <h2>The requested URL was not found on this server.</h2> <h2></h2> </body></html> at com.google.firebase.functions.PublisherStream.validateResponse(PublisherStream.kt:316) at com.google.firebase.functions.PublisherStream.access$validateResponse(PublisherStream.kt:41) at com.google.firebase.functions.PublisherStream$startStreaming$1$4.onResponse(PublisherStream.kt:161) at okhttp3.RealCall$AsyncCall.execute(RealCall.java:203) at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1156) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:651) at java.lang.Thread.run(Thread.java:1119) ```

These improvements are both in the SDK itself, and in the code used for the actual functions running in the cloud during integration testing: A summary of the changes is: - [Backend functions] Update the dependencies and node runtime version - [Backend functions] Add logging in case the client does not support streaming - [SDK] Use a more robust handle of the media type - [SDK] Fix issue introduced in #6773 that missed a return in case of error - [SDK] Other minor code fixes b/404814020

Throwing inside the `PublisherStream` causes a Runtime exception that can't be caught in the call site. Instead, we should use `notifyError()` so that the error can be caught in the `Subscriber#onError()` function. I tried to catch the exception using both of the code snippets below, but none of them worked. (they both work with the changes in this PR): ```kotlin functions.getHttpsCallable("nonExistentFunction") .stream().asFlow() .catch { // Handle error for a 404 function } .collect { // ... } ``` ```kotlin try { functions.getHttpsCallable("nonExistentFunction") .stream().asFlow() .collect { // ... } } catch(e: Exception) { // Handle error for a 404 function } ``` Runtime exception thrown: ``` FATAL EXCEPTION: OkHttp Dispatcher Process: com.google.samples.quickstart.functions, PID: 13321 com.google.firebase.functions.FirebaseFunctionsException: Value <html><head> of type java.lang.String cannot be converted to JSONObject Unexpected Response: <html><head> <meta http-equiv="content-type" content="text/html;charset=utf-8"> <title>404 Page not found</title> </head> <body text=#000000 bgcolor=#ffffff> <h1>Error: Page not found</h1> <h2>The requested URL was not found on this server.</h2> <h2></h2> </body></html> at com.google.firebase.functions.PublisherStream.validateResponse(PublisherStream.kt:316) at com.google.firebase.functions.PublisherStream.access$validateResponse(PublisherStream.kt:41) at com.google.firebase.functions.PublisherStream$startStreaming$1$4.onResponse(PublisherStream.kt:161) at okhttp3.RealCall$AsyncCall.execute(RealCall.java:203) at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1156) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:651) at java.lang.Thread.run(Thread.java:1119) ```

refactor(functions): notifyError() instead of throwing

8352adc

thatfiredev requested a review from emilypgoogle March 14, 2025 13:25

google-oss-bot added the size/M label Mar 14, 2025

rlazo approved these changes Mar 14, 2025

View reviewed changes

style: spotless apply

6b62413

emilypgoogle approved these changes Mar 14, 2025

View reviewed changes

rlazo merged commit 0554f0d into main Mar 14, 2025
31 of 33 checks passed

rlazo deleted the rpf/functions-fix-publisher-errors branch March 14, 2025 21:10

rlazo mentioned this pull request Mar 24, 2025

[Functions] Several functions improvements #6796

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(functions): use notifyError() instead of throwing #6773

fix(functions): use notifyError() instead of throwing #6773

thatfiredev commented Mar 14, 2025 •

edited

Loading

github-actions bot commented Mar 14, 2025 •

edited

Loading

google-oss-bot commented Mar 14, 2025

google-oss-bot commented Mar 14, 2025 •

edited

Loading

`firebase-functions`

github-actions bot commented Mar 14, 2025 •

edited

Loading

google-oss-bot commented Mar 14, 2025 •

edited

Loading

`firebase-functions`

fix(functions): use notifyError() instead of throwing #6773

fix(functions): use notifyError() instead of throwing #6773

Conversation

thatfiredev commented Mar 14, 2025 • edited Loading

github-actions bot commented Mar 14, 2025 • edited Loading

📝 PRs merging into main branch

google-oss-bot commented Mar 14, 2025

google-oss-bot commented Mar 14, 2025 • edited Loading

Coverage Report 1

Affected Products

firebase-functions

Test Logs

github-actions bot commented Mar 14, 2025 • edited Loading

Test Results

google-oss-bot commented Mar 14, 2025 • edited Loading

Size Report 1

Affected Products

firebase-functions

Test Logs

thatfiredev commented Mar 14, 2025 •

edited

Loading

github-actions bot commented Mar 14, 2025 •

edited

Loading

google-oss-bot commented Mar 14, 2025 •

edited

Loading

Coverage Report ¹

`firebase-functions`

github-actions bot commented Mar 14, 2025 •

edited

Loading

google-oss-bot commented Mar 14, 2025 •

edited

Loading

Size Report ¹

`firebase-functions`