diff --git a/android/app/src/main/java/com/anonymous/bulwarkmobile/BulwarkMessagingService.kt b/android/app/src/main/java/com/anonymous/bulwarkmobile/BulwarkMessagingService.kt index 27c567a8..f29f3909 100644 --- a/android/app/src/main/java/com/anonymous/bulwarkmobile/BulwarkMessagingService.kt +++ b/android/app/src/main/java/com/anonymous/bulwarkmobile/BulwarkMessagingService.kt @@ -66,7 +66,12 @@ class BulwarkMessagingService : FirebaseMessagingService() { fun startTaskUnlessInForeground(context: Context, data: Map, mail: Boolean) { val foreground = MainActivity.isInForeground Log.i(TAG, "Push: mail=$mail foreground=$foreground importance=${processImportance()}") - if (mail && !foreground) startHeadlessTask(context, data) + if (mail && !foreground) { + // The task opens its own connection rather than one left + // over from the foreground, which may be dead (#94). + HttpConnections.dropIdle() + startHeadlessTask(context, data) + } } /** For the log only. */ diff --git a/android/app/src/main/java/com/anonymous/bulwarkmobile/HttpConnections.kt b/android/app/src/main/java/com/anonymous/bulwarkmobile/HttpConnections.kt new file mode 100644 index 00000000..b0bf1a98 --- /dev/null +++ b/android/app/src/main/java/com/anonymous/bulwarkmobile/HttpConnections.kt @@ -0,0 +1,41 @@ +package com.anonymous.bulwarkmobile + +import android.content.Context +import com.facebook.react.modules.network.OkHttpClientFactory +import com.facebook.react.modules.network.OkHttpClientProvider +import okhttp3.ConnectionPool +import okhttp3.OkHttpClient + +/** + * One connection pool for every OkHttp client React Native builds (fetch, + * XMLHttpRequest and the event stream on top of it), so the push path can + * drop the connections the app kept from the foreground. + * + * React Native's client has no connect or read timeout. A connection that + * died silently while the app was in the background (a mobile network's NAT + * forgot it, the server closed it and the FIN never arrived) stays in the + * pool, and the push task's first request was sent down it and waited until + * the task's 30 s timeout: no request reached the server and no + * notification appeared (#94). A fresh connection costs one TLS handshake. + */ +object HttpConnections { + private val pool = ConnectionPool() + + /** Call before React Native creates its first network module. */ + fun install(context: Context) { + val app = context.applicationContext + OkHttpClientProvider.setOkHttpClientFactory( + object : OkHttpClientFactory { + override fun createNewNetworkModuleClient(): OkHttpClient = + OkHttpClientProvider.createClientBuilder(app) + .connectionPool(pool) + .build() + }, + ) + } + + /** Closes every pooled connection no request is using. */ + fun dropIdle() { + pool.evictAll() + } +} diff --git a/android/app/src/main/java/com/anonymous/bulwarkmobile/MainApplication.kt b/android/app/src/main/java/com/anonymous/bulwarkmobile/MainApplication.kt index 7dc526bc..3fa5e6c5 100644 --- a/android/app/src/main/java/com/anonymous/bulwarkmobile/MainApplication.kt +++ b/android/app/src/main/java/com/anonymous/bulwarkmobile/MainApplication.kt @@ -40,6 +40,7 @@ class MainApplication : Application(), ReactApplication { override fun onCreate() { super.onCreate() + HttpConnections.install(this) DefaultNewArchitectureEntryPoint.releaseLevel = try { ReleaseLevel.valueOf(BuildConfig.REACT_NATIVE_RELEASE_LEVEL.uppercase()) } catch (e: IllegalArgumentException) { diff --git a/src/lib/__tests__/push-background-task.test.ts b/src/lib/__tests__/push-background-task.test.ts index 6f3bf43e..6ed3c19d 100644 --- a/src/lib/__tests__/push-background-task.test.ts +++ b/src/lib/__tests__/push-background-task.test.ts @@ -19,6 +19,7 @@ import { handleNotificationAction, matchAccountsForPush, parseRelayPushData, + PUSH_REQUEST_TIMEOUT_MS, pushBackgroundTask, selectNotifiableEmails, senderFaviconsAllowed, @@ -268,6 +269,30 @@ describe('pushBackgroundTask notifications', () => { }); }); + it('shows the generic notification when a request never answers, as on a dead connection (#94)', async () => { + vi.useFakeTimers(); + try { + (secureFetch as ReturnType).mockImplementation(() => new Promise(() => undefined)); + let settled = false; + const task = pushBackgroundTask({ kind: 'jmap-email-push', accountLabel: 'alice', accountId: 'jmap-primary', emailIds: '["m1"]' }) + .then(() => { settled = true; }); + + await vi.advanceTimersByTimeAsync(PUSH_REQUEST_TIMEOUT_MS - 1); + expect(settled).toBe(false); + expect(showNotification).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(1); + await task; + expect(showNotification).toHaveBeenCalledTimes(1); + expect((showNotification.mock.calls[0] as unknown[])[0]).toMatchObject({ title: 'New email', body: 'alice', emailId: '' }); + // The request was told to stop, so it doesn't hold the connection. + const init = (secureFetch as ReturnType).mock.calls[0][1] as { signal: AbortSignal }; + expect(init.signal.aborted).toBe(true); + } finally { + vi.useRealTimers(); + } + }); + it('takes a method error for a failed lookup, not for "nothing new" (#1041)', async () => { (secureFetch as ReturnType).mockImplementation(async (url: string) => { if (url.endsWith('/.well-known/jmap')) { diff --git a/src/lib/push-background-task.ts b/src/lib/push-background-task.ts index 283247ae..cd1a8ce8 100644 --- a/src/lib/push-background-task.ts +++ b/src/lib/push-background-task.ts @@ -321,13 +321,42 @@ function rewriteApiUrl(session: JMAPSession, serverUrl: string): string { return serverOrigin + session.apiUrl.slice(apiOrigin.length); } +/** + * How long one request of the push task may go unanswered. React Native's + * HTTP client has no timeout of its own, so a request on a dead connection + * waited until the task's 30 s were up and ended without any notification + * (#94). This leaves time for the session and the lookup, and the failure + * still shows the generic one. + */ +export const PUSH_REQUEST_TIMEOUT_MS = 10_000; + +/** secureFetch that gives up after PUSH_REQUEST_TIMEOUT_MS. */ +async function fetchInTime(url: string, init: RequestInit): Promise { + const controller = new AbortController(); + let timer: ReturnType | undefined; + const deadline = new Promise((_, reject) => { + timer = setTimeout(() => { + controller.abort(); + reject(new PushLookupError(`No answer within ${PUSH_REQUEST_TIMEOUT_MS} ms`)); + }, PUSH_REQUEST_TIMEOUT_MS); + }); + try { + return await Promise.race([ + secureFetch(url, { ...init, signal: controller.signal, timeoutMs: PUSH_REQUEST_TIMEOUT_MS }), + deadline, + ]); + } finally { + clearTimeout(timer); + } +} + async function openDetachedSession(accountId: string): Promise { let creds = await jmapClient.getStoredCredentials(accountId); if (!creds) return null; creds = await freshStoredCredentials(accountId, creds); const baseUrl = creds.serverUrl.replace(/\/+$/, ''); const authHeader = authHeaderFor(creds); - const res = await secureFetch(`${baseUrl}/.well-known/jmap`, { + const res = await fetchInTime(`${baseUrl}/.well-known/jmap`, { headers: { Authorization: authHeader, Accept: 'application/json' }, }); if (!res.ok) throw new Error(`Session discovery failed: ${res.status}`); @@ -370,7 +399,7 @@ async function jmapPost( session: DetachedSession, methodCalls: JMAPMethodCall[], ): Promise, string]>> { - const response = await secureFetch(session.apiUrl, { + const response = await fetchInTime(session.apiUrl, { method: 'POST', headers: { 'Content-Type': 'application/json', Authorization: session.authHeader }, body: JSON.stringify({ using: [CAPABILITIES.CORE, CAPABILITIES.MAIL], methodCalls }),