From 19193bbedfd907050d03a21c9291284d046190d7 Mon Sep 17 00:00:00 2001 From: feruzm Date: Mon, 31 Aug 2026 13:16:08 +0000 Subject: [PATCH 1/3] fix(network): bound every request and surface the failure Requests on the API path carried no deadline, so a connection that was accepted and then went quiet never produced a result. React Native builds its Android OkHttpClient with connect, read and write timeouts of 0, the whatwg-fetch polyfill never sets xhr.timeout, and axios picks its xhr adapter when XMLHttpRequest is defined, so it bypasses the fetch path entirely and an instance with no timeout inherits 0. The promise never settled, React Query never left pending, and the screen kept its skeleton with no error and no retry. OkHttp compounds it: the dispatcher allows five concurrent calls per host, so five stalled calls park every later call to that host behind them for the life of the process. utils/networkTimeout wraps the global fetch. 20s for our own hosts, 30s elsewhere, 120s when the body is an upload, and no deadline at all for a non-HTTP scheme, so the file:// read behind a picked video is untouched. It is installed from index.js before ./App, because @ecency/sdk binds globalThis.fetch on first use and caches the bound reference. An expired request rejects with name 'TimeoutError', which is the contract the retry policy and the error view read. Every axios instance gets an explicit timeout. purchaseOrder overrides it with a wider one: it is not idempotent, and a client deadline there turns a slow success into a charge whose outcome nobody knows. ecencyApi renames an expired request through a response interceptor, and config/axiosTimeout does the classifying, since Android reports an expired callTimeout as a generic transport failure rather than as a timeout. The QueryClient retries once, only for statuses where a retry can help, and never for a query React Query cancelled itself. networkMode is 'always' on queries and mutations, because a paused query is indistinguishable from a loading one in the UI. NetInfo drives onlineManager instead, so refetchOnReconnect recovers a failed screen once the network returns. QueryErrorRetry is the terminal state a failed query had nowhere to render. It replaces the indefinite skeleton in the feed tabs and in notifications, and the quests card that used to draw every quest at 0/goal when its request failed. Both list call sites scope the error to the first page, so a failed "load more" cannot wipe content already on screen. MainApplication.kt sets connect 10s and read/write 30s on the shared OkHttpClient as a backstop for whatever reaches the network without a JS deadline; the per-request deadline stays on callTimeout. sdk-config now applies the hive-tx timeout and the saved node before its first await, so both are in place for the calls that follow rather than after them. Transport failures are no longer reported from the analytics and points paths. Both run on every screen view or user action and nothing waits on their result, so with a deadline in place they would report one event per call for work the user never sees. --- .../esteem/mobile/android/MainApplication.kt | 36 +++ index.js | 3 + src/components/basicUIElements/index.tsx | 2 + .../queryErrorRetry/queryErrorRetryStyles.ts | 51 ++++ .../queryErrorRetryView.test.tsx | 64 +++++ .../queryErrorRetry/queryErrorRetryView.tsx | 62 +++++ src/components/index.tsx | 2 + .../notification/view/notificationView.tsx | 43 ++- .../tabbedPosts/view/listEmptyView.tsx | 26 +- .../tabbedPosts/view/postsTabContent.tsx | 13 +- src/config/axiosTimeout.test.ts | 111 ++++++++ src/config/axiosTimeout.ts | 68 +++++ src/config/coingeckoApi.ts | 7 + src/config/ecencyApi.ts | 36 ++- src/config/githubApi.ts | 6 + src/config/locales/en-US.json | 3 + src/config/translationApi.ts | 7 + src/providers/ecency/ePoint.ts | 10 +- src/providers/ecency/ecency.ts | 15 +- src/providers/plausible/plausible.ts | 18 +- src/providers/queries/index.ts | 45 ++- src/providers/queries/notificationQueries.ts | 4 + .../queries/postQueries/feedQueries.ts | 10 + src/providers/queries/retryPolicy.test.ts | 54 ++++ src/providers/queries/retryPolicy.ts | 52 ++++ src/providers/queries/sdk-config.ts | 26 +- .../container/notificationContainer.tsx | 2 + .../screen/notificationScreen.tsx | 4 + src/screens/perks/children/questsCard.tsx | 16 +- src/utils/abortSignalPolyfill.test.ts | 14 + src/utils/abortSignalPolyfill.ts | 18 +- src/utils/installFetchDeadline.test.ts | 67 +++++ src/utils/installFetchDeadline.ts | 30 ++ src/utils/networkTimeout.test.ts | 245 ++++++++++++++++ src/utils/networkTimeout.ts | 261 ++++++++++++++++++ 35 files changed, 1398 insertions(+), 33 deletions(-) create mode 100644 src/components/basicUIElements/view/queryErrorRetry/queryErrorRetryStyles.ts create mode 100644 src/components/basicUIElements/view/queryErrorRetry/queryErrorRetryView.test.tsx create mode 100644 src/components/basicUIElements/view/queryErrorRetry/queryErrorRetryView.tsx create mode 100644 src/config/axiosTimeout.test.ts create mode 100644 src/config/axiosTimeout.ts create mode 100644 src/providers/queries/retryPolicy.test.ts create mode 100644 src/providers/queries/retryPolicy.ts create mode 100644 src/utils/abortSignalPolyfill.test.ts create mode 100644 src/utils/installFetchDeadline.test.ts create mode 100644 src/utils/installFetchDeadline.ts create mode 100644 src/utils/networkTimeout.test.ts create mode 100644 src/utils/networkTimeout.ts diff --git a/android/app/src/main/java/app/esteem/mobile/android/MainApplication.kt b/android/app/src/main/java/app/esteem/mobile/android/MainApplication.kt index ac0004b556..6d4e09b982 100644 --- a/android/app/src/main/java/app/esteem/mobile/android/MainApplication.kt +++ b/android/app/src/main/java/app/esteem/mobile/android/MainApplication.kt @@ -9,8 +9,11 @@ import com.facebook.react.ReactPackage import com.facebook.react.defaults.DefaultNewArchitectureEntryPoint.load import com.facebook.react.defaults.DefaultReactHost.getDefaultReactHost import com.facebook.react.defaults.DefaultReactNativeHost +import com.facebook.react.modules.network.OkHttpClientFactory +import com.facebook.react.modules.network.OkHttpClientProvider import com.facebook.react.soloader.OpenSourceMergedSoMapping import com.facebook.soloader.SoLoader +import java.util.concurrent.TimeUnit //expo related packages import android.content.Context @@ -49,6 +52,39 @@ class MainApplication : Application(), ReactApplication { override fun onCreate() { super.onCreate() + // React Native builds its shared OkHttpClient with connect, read and write + // timeouts of 0, which OkHttp reads as "wait forever" + // (OkHttpClientProvider.createClientBuilder). A socket that is accepted and + // then goes silent therefore never produces an error, and the JS promise + // behind it never settles. + // + // connectTimeout is the one that changes behaviour rather than just adding a + // ceiling: OkHttp tries a host's addresses one route at a time, each with its + // own connect timeout, so with 0 a single black-holed address hangs the call + // forever and the remaining addresses are never tried. 10s is well above a + // real handshake even on a poor link, and low enough that a dead route falls + // over to the next one inside the JS deadline (utils/networkTimeout). + // + // read and write are idle timeouts, not deadlines: readTimeout bounds the gap + // between bytes and writeTimeout the gap between writes, so a slow but + // progressing upload is unaffected and a large download still completes. + // The per-request deadline stays on callTimeout, which React Native sets per + // request from the JS-side timeout (NetworkingModule). + // + // A backstop under the JS-side deadlines, not a replacement for them: it + // catches whatever reaches the network without one. Must be set before the + // first client is created, which happens when NetworkingModule is built. + OkHttpClientProvider.setOkHttpClientFactory( + OkHttpClientFactory { + // createClientBuilder(context) keeps React Native's own cookie jar + // and its 10MB response cache; only the timeouts change. + OkHttpClientProvider.createClientBuilder(this) + .connectTimeout(10, TimeUnit.SECONDS) + .readTimeout(30, TimeUnit.SECONDS) + .writeTimeout(30, TimeUnit.SECONDS) + .build() + } + ) SoLoader.init(this, OpenSourceMergedSoMapping) diff --git a/index.js b/index.js index 747f630811..b4c6ccce5d 100644 --- a/index.js +++ b/index.js @@ -6,6 +6,9 @@ import 'intl'; import 'intl/locale-data/jsonp/en-US'; import 'react-native-get-random-values'; import './src/utils/abortSignalPolyfill'; +// Bounds every HTTP(S) fetch. Must run before ./App, which pulls in @ecency/sdk: +// the SDK binds globalThis.fetch on first use and caches the bound reference. +import './src/utils/installFetchDeadline'; import EcencyApp from './App'; diff --git a/src/components/basicUIElements/index.tsx b/src/components/basicUIElements/index.tsx index a530b53e98..a40ebbdf40 100644 --- a/src/components/basicUIElements/index.tsx +++ b/src/components/basicUIElements/index.tsx @@ -12,6 +12,7 @@ import WalletLineItem from './view/walletLineItem/walletLineItemView'; import CommunityListItem from './view/communityListItem/communityListItem'; import Separator from './view/separator/separatorView'; import EmptyScreen from './view/emptyScreen/emptyScreenView'; +import QueryErrorRetry from './view/queryErrorRetry/queryErrorRetryView'; // // Placeholders import ListItemPlaceHolder from './view/placeHolder/listItemPlaceHolderView'; @@ -52,4 +53,5 @@ export { CommunitiesPlaceHolder, Separator, EmptyScreen, + QueryErrorRetry, }; diff --git a/src/components/basicUIElements/view/queryErrorRetry/queryErrorRetryStyles.ts b/src/components/basicUIElements/view/queryErrorRetry/queryErrorRetryStyles.ts new file mode 100644 index 0000000000..0c8261f677 --- /dev/null +++ b/src/components/basicUIElements/view/queryErrorRetry/queryErrorRetryStyles.ts @@ -0,0 +1,51 @@ +import EStyleSheet from 'react-native-extended-stylesheet'; + +export default EStyleSheet.create({ + container: { + alignItems: 'center', + justifyContent: 'center', + paddingHorizontal: 32, + paddingVertical: 48, + }, + containerCompact: { + alignItems: 'center', + justifyContent: 'center', + paddingHorizontal: 16, + paddingVertical: 20, + }, + icon: { + color: '$iconColor', + marginBottom: 8, + }, + message: { + color: '$primaryDarkText', + fontFamily: '$primaryFont', + fontSize: 14, + marginBottom: 16, + textAlign: 'center', + }, + messageCompact: { + color: '$primaryDarkGray', + fontFamily: '$primaryFont', + fontSize: 13, + marginBottom: 12, + textAlign: 'center', + }, + button: { + alignItems: 'center', + backgroundColor: '$primaryBlue', + borderRadius: 20, + height: 40, + justifyContent: 'center', + paddingHorizontal: 24, + }, + buttonDisabled: { + opacity: 0.6, + }, + buttonText: { + color: '$pureWhite', + fontFamily: '$primaryFont', + fontSize: 13, + fontWeight: '600', + }, +}); diff --git a/src/components/basicUIElements/view/queryErrorRetry/queryErrorRetryView.test.tsx b/src/components/basicUIElements/view/queryErrorRetry/queryErrorRetryView.test.tsx new file mode 100644 index 0000000000..b4daefb9bf --- /dev/null +++ b/src/components/basicUIElements/view/queryErrorRetry/queryErrorRetryView.test.tsx @@ -0,0 +1,64 @@ +import React from 'react'; +import TestRenderer from 'react-test-renderer'; +import { Text, TouchableOpacity } from 'react-native'; + +jest.mock('react-native-extended-stylesheet', () => ({ + create: (styles: any) => styles, + value: jest.fn(() => '#000000'), +})); + +// Icon pulls in react-native-vector-icons' native font loading. +jest.mock('../../../icon', () => ({ Icon: 'Icon' })); + +jest.mock('react-intl', () => ({ + useIntl: () => ({ formatMessage: ({ id }: { id: string }) => id }), +})); + +// eslint-disable-next-line import/first +import QueryErrorRetry from './queryErrorRetryView'; + +const render = (props: React.ComponentProps) => { + let tree!: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + tree = TestRenderer.create(); + }); + return tree; +}; + +const messages = (tree: TestRenderer.ReactTestRenderer) => + tree.root.findAllByType(Text as any).map((node) => node.props.children); + +describe('QueryErrorRetry', () => { + it('tells a timeout apart from any other failure', () => { + const timeout = Object.assign(new Error('Request timed out'), { name: 'TimeoutError' }); + + expect(messages(render({ error: timeout, onRetry: jest.fn() }))).toContain( + 'alert.request_timed_out', + ); + expect(messages(render({ error: new Error('boom'), onRetry: jest.fn() }))).toContain( + 'alert.load_failed_retry', + ); + }); + + it('says it is retrying while the retry is in flight, and disables the button', () => { + const tree = render({ onRetry: jest.fn(), isRetrying: true }); + + expect(messages(tree)).toContain('alert.retrying'); + expect(tree.root.findByType(TouchableOpacity as any).props.disabled).toBe(true); + }); + + it('calls onRetry with no arguments', () => { + // Load-bearing: `refetch` is passed straight in at some call sites, and + // React Query reads its first argument as options. Handing it the press + // event would be interpreted as a refetch configuration object. + const onRetry = jest.fn(); + const tree = render({ onRetry }); + + TestRenderer.act(() => { + tree.root.findByType(TouchableOpacity as any).props.onPress({ nativeEvent: {} }); + }); + + expect(onRetry).toHaveBeenCalledTimes(1); + expect(onRetry).toHaveBeenCalledWith(); + }); +}); diff --git a/src/components/basicUIElements/view/queryErrorRetry/queryErrorRetryView.tsx b/src/components/basicUIElements/view/queryErrorRetry/queryErrorRetryView.tsx new file mode 100644 index 0000000000..6025c4d400 --- /dev/null +++ b/src/components/basicUIElements/view/queryErrorRetry/queryErrorRetryView.tsx @@ -0,0 +1,62 @@ +import React from 'react'; +import { Text, TouchableOpacity, View } from 'react-native'; +import { useIntl } from 'react-intl'; + +import { Icon } from '../../../icon'; +import styles from './queryErrorRetryStyles'; + +interface Props { + /** The query error, used only to pick between the two messages. */ + error?: unknown; + onRetry: () => void; + /** True while the retry is in flight, so the button reads as busy. */ + isRetrying?: boolean; + /** Inline variant for a card or a list header rather than a full empty state. */ + compact?: boolean; +} + +/** + * Terminal state for a query that failed: says what happened and offers the one + * action that can fix it. Every list or card that can show a loading skeleton + * needs one of these, otherwise a request that never answers reads as a screen + * that is still working. + * + * `TimeoutError` is set by the global fetch deadline (utils/networkTimeout) and + * by the ecencyApi response interceptor, and it earns a different message: the + * server said nothing at all, which points at the connection rather than at us. + */ +const QueryErrorRetry = ({ error, onRetry, isRetrying, compact }: Props) => { + const intl = useIntl(); + + const isTimeout = (error as { name?: string })?.name === 'TimeoutError'; + + return ( + + + + {intl.formatMessage({ + id: isTimeout ? 'alert.request_timed_out' : 'alert.load_failed_retry', + })} + + onRetry()} + disabled={isRetrying} + accessibilityRole="button" + > + + {intl.formatMessage({ + id: isRetrying ? 'alert.retrying' : 'alert.something_wrong_reload', + })} + + + + ); +}; + +export default QueryErrorRetry; diff --git a/src/components/index.tsx b/src/components/index.tsx index 0978d2d401..fab687941d 100644 --- a/src/components/index.tsx +++ b/src/components/index.tsx @@ -148,6 +148,7 @@ import { PostCardPlaceHolder, PostPlaceHolder, ProfileSummaryPlaceHolder, + QueryErrorRetry, StickyBar, Tag, TextWithIcon, @@ -226,6 +227,7 @@ export { ProfileSummary, ProfileSummaryPlaceHolder, Promote, + QueryErrorRetry, PulseAnimation, ScaleSlider, SearchInput, diff --git a/src/components/notification/view/notificationView.tsx b/src/components/notification/view/notificationView.tsx index d237936335..fd1f11185e 100644 --- a/src/components/notification/view/notificationView.tsx +++ b/src/components/notification/view/notificationView.tsx @@ -6,7 +6,7 @@ import { ActivityIndicator, FlatList, Text, View, RefreshControl } from 'react-n // Components import EStyleSheet from 'react-native-extended-stylesheet'; import { NotificationLine } from '../..'; -import { ListPlaceHolder } from '../../basicUIElements'; +import { ListPlaceHolder, QueryErrorRetry } from '../../basicUIElements'; import { FilterBar } from '../../filterBar'; // Styles @@ -42,6 +42,9 @@ interface Props { notifications: any[]; isLoading: boolean; isFetching: boolean; + /** The list failed and there is nothing cached to show instead. */ + isError?: boolean; + error?: unknown; isNotificationRefreshing: boolean; globalProps: any; handleOnUserPress: (username?: string) => void; @@ -56,6 +59,8 @@ const NotificationView = ({ notifications, isLoading, isFetching, + isError, + error, isNotificationRefreshing, globalProps, handleOnUserPress, @@ -109,6 +114,32 @@ const NotificationView = ({ return null; }; + // Order matters: the failure is checked before the loading skeleton, because + // a query that failed is still `isFetching` for the moment React Query spends + // settling it, and before the "no activity" copy, which would otherwise claim + // an empty inbox on a request that never arrived. + const _renderEmptyComponent = () => { + if (isError) { + return ( + getActivities()} + isRetrying={isNotificationRefreshing} + /> + ); + } + + if (isLoading || isFetching || isNotificationRefreshing) { + return ; + } + + return ( + + {intl.formatMessage({ id: 'notification.noactivity' })} + + ); + }; + const _renderItem = ({ item }: any) => ( - ) : ( - - {intl.formatMessage({ id: 'notification.noactivity' })} - - ) - } + ListEmptyComponent={_renderEmptyComponent} contentContainerStyle={styles.listContentContainer} refreshControl={ void; } -const TabEmptyView = ({ filterKey, isNoPost }: TabEmptyViewProps) => { +const TabEmptyView = ({ + filterKey, + isNoPost, + isError, + // Renamed on the way in: this component already destructures an `error` out of + // the leaderboard and communities redux slices further down. + error: loadError, + isRetrying, + onRetry, +}: TabEmptyViewProps) => { const intl = useIntl(); const dispatch = useDispatch(); const navigation = useNavigation(); @@ -302,6 +316,14 @@ const TabEmptyView = ({ filterKey, isNoPost }: TabEmptyViewProps) => { } } + // Checked after the logged-out and empty-feed branches, both of which are + // real answers rather than failures, and before the placeholder: the + // placeholder is the fallthrough for "still loading", so without this a feed + // whose first page failed keeps a skeleton on screen with no way forward. + if (isError && onRetry) { + return ; + } + return ( diff --git a/src/components/tabbedPosts/view/postsTabContent.tsx b/src/components/tabbedPosts/view/postsTabContent.tsx index d8f34c416d..0819f35854 100644 --- a/src/components/tabbedPosts/view/postsTabContent.tsx +++ b/src/components/tabbedPosts/view/postsTabContent.tsx @@ -155,8 +155,17 @@ const PostsTabContent = ({ // view rendereres const _renderEmptyContent = () => { - const _isNoPost = !feedQuery.isLoading && feedQuery.data.length == 0; - return ; + const _isNoPost = !feedQuery.isLoading && !feedQuery.isError && feedQuery.data.length == 0; + return ( + + ); }; const scrollPopupCallback = useCallback((value: boolean) => { diff --git a/src/config/axiosTimeout.test.ts b/src/config/axiosTimeout.test.ts new file mode 100644 index 0000000000..c450270154 --- /dev/null +++ b/src/config/axiosTimeout.test.ts @@ -0,0 +1,111 @@ +import { AxiosError, InternalAxiosRequestConfig } from 'axios'; +import { + TimedRequestConfig, + isAxiosTimeoutError, + isAxiosTransportError, + stampRequestStart, +} from './axiosTimeout'; + +const makeConfig = (timeout: number, startedAt?: number): TimedRequestConfig => + ({ + timeout, + ...(startedAt === undefined ? {} : { metadata: { startedAt } }), + } as TimedRequestConfig); + +const makeError = (code: string, config?: InternalAxiosRequestConfig) => + new AxiosError('failed', code, config); + +describe('stampRequestStart', () => { + it('records when the request left', () => { + jest.spyOn(Date, 'now').mockReturnValue(1_000_000); + const request = {} as InternalAxiosRequestConfig; + + const result = stampRequestStart(request); + + expect(result).toBe(request); + expect((request as TimedRequestConfig).metadata).toEqual({ startedAt: 1_000_000 }); + jest.restoreAllMocks(); + }); +}); + +describe('isAxiosTimeoutError', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('recognises the code iOS produces for an expired deadline', () => { + expect(isAxiosTimeoutError(makeError('ECONNABORTED'))).toBe(true); + }); + + it('recognises the clarified timeout code', () => { + expect(isAxiosTimeoutError(makeError('ETIMEDOUT'))).toBe(true); + }); + + it('recognises the generic transport failure Android reports for an expired deadline', () => { + jest.spyOn(Date, 'now').mockReturnValue(21_000); + const error = makeError('ERR_NETWORK', makeConfig(20000, 1_000)); + + expect(isAxiosTimeoutError(error)).toBe(true); + }); + + it('accepts a native deadline that fired just before this read of the clock', () => { + // The elapsed check must allow a little slack, or a timeout that fires a few + // milliseconds early is reported as a plain connection failure. + jest.spyOn(Date, 'now').mockReturnValue(20_900); + const error = makeError('ERR_NETWORK', makeConfig(20000, 1_000)); + + expect(isAxiosTimeoutError(error)).toBe(true); + }); + + it('does not call a fast connection failure a timeout', () => { + // The case that makes the elapsed check necessary: offline fails instantly + // with the same code an expired Android deadline produces. + jest.spyOn(Date, 'now').mockReturnValue(1_300); + const error = makeError('ERR_NETWORK', makeConfig(20000, 1_000)); + + expect(isAxiosTimeoutError(error)).toBe(false); + }); + + it('does not guess when the request carried no deadline', () => { + jest.spyOn(Date, 'now').mockReturnValue(999_999); + const error = makeError('ERR_NETWORK', makeConfig(0, 1_000)); + + expect(isAxiosTimeoutError(error)).toBe(false); + }); + + it('does not guess when the request was never stamped', () => { + jest.spyOn(Date, 'now').mockReturnValue(999_999); + const error = makeError('ERR_NETWORK', makeConfig(20000)); + + expect(isAxiosTimeoutError(error)).toBe(false); + }); + + it.each([ + ['a server answer', 'ERR_BAD_REQUEST'], + ['a cancelled request', 'ERR_CANCELED'], + ])('leaves %s alone', (_label, code) => { + expect(isAxiosTimeoutError(makeError(code))).toBe(false); + }); + + it('ignores anything that is not an axios error', () => { + expect(isAxiosTimeoutError(new Error('ECONNABORTED'))).toBe(false); + expect(isAxiosTimeoutError(undefined)).toBe(false); + }); +}); + +describe('isAxiosTransportError', () => { + it.each([['ECONNABORTED'], ['ETIMEDOUT'], ['ERR_NETWORK']])( + 'treats %s as a failure where no response arrived', + (code) => { + expect(isAxiosTransportError(makeError(code))).toBe(true); + }, + ); + + it('does not treat a server answer as a transport failure', () => { + expect(isAxiosTransportError(makeError('ERR_BAD_RESPONSE'))).toBe(false); + }); + + it('ignores anything that is not an axios error', () => { + expect(isAxiosTransportError(new Error('ERR_NETWORK'))).toBe(false); + }); +}); diff --git a/src/config/axiosTimeout.ts b/src/config/axiosTimeout.ts new file mode 100644 index 0000000000..623f891063 --- /dev/null +++ b/src/config/axiosTimeout.ts @@ -0,0 +1,68 @@ +import axios, { AxiosError, InternalAxiosRequestConfig } from 'axios'; + +/** Request config with the start stamp the timeout classifier below needs. */ +export type TimedRequestConfig = InternalAxiosRequestConfig & { metadata?: { startedAt: number } }; + +/** + * Request interceptor: stamp the moment the request left, so a response error can + * be measured against the configured deadline. See `isAxiosTimeoutError`. + */ +export const stampRequestStart = (request: T): T => { + (request as TimedRequestConfig).metadata = { startedAt: Date.now() }; + return request; +}; + +/** + * Slack for the gap between the native deadline firing and this JS read of the + * clock. Generous on purpose: mislabelling a real timeout as a network error is + * the failure that matters here, and the elapsed-time check is only ever reached + * for a request that already carried a deadline. + */ +const ELAPSED_SLACK_MS = 250; + +/** + * True when the request ended because its own deadline expired, rather than + * because the server answered or the connection failed outright. + * + * iOS reports a timeout as a timeout and axios produces 'ECONNABORTED' + * ('ETIMEDOUT' when `clarifyTimeoutError` is on). Android does not: React Native + * flags the XHR `timeout` event only when the native failure class is exactly + * SocketTimeoutException (ResponseUtil.onRequestError), and an expired OkHttp + * callTimeout raises InterruptedIOException instead, so axios sees a generic + * transport failure and produces 'ERR_NETWORK'. Recognising 'ERR_NETWORK' alone + * would relabel every offline failure as a timeout, so it is only accepted when + * the request also ran for as long as it was allowed to. + */ +export const isAxiosTimeoutError = (error: unknown): error is AxiosError => { + if (!axios.isAxiosError(error)) { + return false; + } + if (error.code === 'ECONNABORTED' || error.code === 'ETIMEDOUT') { + return true; + } + if (error.code !== 'ERR_NETWORK') { + return false; + } + + const config = error.config as TimedRequestConfig | undefined; + const limit = config?.timeout ?? 0; + const startedAt = config?.metadata?.startedAt; + if (!limit || !startedAt) { + return false; + } + return Date.now() - startedAt >= limit - ELAPSED_SLACK_MS; +}; + +/** + * True for any failure where no HTTP response ever arrived: a timeout or a + * transport error. Callers that report errors use this to keep a broken network + * path from filling crash reporting with one event per request. + */ +export const isAxiosTransportError = (error: unknown): boolean => { + if (!axios.isAxiosError(error)) { + return false; + } + return ( + error.code === 'ECONNABORTED' || error.code === 'ETIMEDOUT' || error.code === 'ERR_NETWORK' + ); +}; diff --git a/src/config/coingeckoApi.ts b/src/config/coingeckoApi.ts index d02d39bd32..7addae80c9 100644 --- a/src/config/coingeckoApi.ts +++ b/src/config/coingeckoApi.ts @@ -1,5 +1,7 @@ import axios from 'axios'; +import { DEFAULT_TIMEOUT_MS } from '../utils/networkTimeout'; + const BASE_URL = 'https://api.coingecko.com'; const PATH_API = 'api'; const API_VERSION = 'v3'; @@ -8,6 +10,11 @@ const API_VERSION = 'v3'; const coingeckoApi = axios.create({ baseURL: `${BASE_URL}/${PATH_API}/${API_VERSION}`, + // Axios bypasses the global fetch deadline (it uses its xhr adapter) and + // defaults to no timeout of its own, so without this a stalled market-data + // request never settles. Market data is decoration on the wallet screen; it + // must never be the reason a request slot stays occupied. + timeout: DEFAULT_TIMEOUT_MS, }); export default coingeckoApi; diff --git a/src/config/ecencyApi.ts b/src/config/ecencyApi.ts index ea0251f397..2a4bf5f8f9 100644 --- a/src/config/ecencyApi.ts +++ b/src/config/ecencyApi.ts @@ -7,6 +7,8 @@ import { store } from '../redux/store/store'; import { getDigitPinCode } from '../providers/hive/hive'; import { decryptKey } from '../utils/crypto'; import { selectIsLoggedIn } from '../redux/selectors'; +import { FIRST_PARTY_TIMEOUT_MS } from '../utils/networkTimeout'; +import { isAxiosTimeoutError, stampRequestStart } from './axiosTimeout'; export const ECENCY_TERMS_URL = `${Config.ECENCY_BACKEND_API}/terms-of-service`; @@ -16,11 +18,25 @@ const ecencyApi = axios.create({ 'Content-Type': 'application/json', 'User-Agent': `${Config.USER_AGENT}/${VersionNumber.appVersion}`, }, + // Axios does not go through the global fetch wrapper: with XMLHttpRequest + // defined it picks its xhr adapter, and an instance with no `timeout` inherits + // 0, which React Native passes through as "no deadline". A request that is + // accepted and never answered then never settles, and it holds one of the five + // concurrent slots the platform HTTP client allows per host for the life of the + // process. Five of those lock out every later call to this host, fetch included. + // + // Same budget as the fetch deadline for this host, and for the same reasons; + // per-request overrides win where an endpoint is not idempotent. + timeout: FIRST_PARTY_TIMEOUT_MS, }); ecencyApi.interceptors.request.use((request) => { // console.log(`Starting ecency Request`, request); + // Stamp the start so the response interceptor can recognise a timeout on + // Android, where the platform reports one as a generic transport failure. + stampRequestStart(request); + // skip code addition is register and token refresh endpoint is triggered if ( request.url === '/private-api/account-create' || @@ -70,9 +86,21 @@ ecencyApi.interceptors.request.use((request) => { return request; }); -ecencyApi.interceptors.response.use((response) => { - // console.log('Response:', response); - return response; -}); +ecencyApi.interceptors.response.use( + (response) => { + // console.log('Response:', response); + return response; + }, + (error) => { + // Rename so callers, the retry policy and the error view can tell "the + // network never answered" from "the server said no". `error.code` is + // deliberately left alone: call sites already branch on it. `error.message` + // is left alone too, because it is surfaced to users. + if (isAxiosTimeoutError(error)) { + error.name = 'TimeoutError'; + } + return Promise.reject(error); + }, +); export default ecencyApi; diff --git a/src/config/githubApi.ts b/src/config/githubApi.ts index e5c54b8e5a..f6d4558b64 100644 --- a/src/config/githubApi.ts +++ b/src/config/githubApi.ts @@ -1,7 +1,13 @@ import axios from 'axios'; +import { DEFAULT_TIMEOUT_MS } from '../utils/networkTimeout'; + const githubApi = axios.create({ baseURL: 'https://api.github.com/repos/ecency/vision-mobile/', + // Only used for the update check on launch. Nothing waits on it, so a stalled + // call would sit open unnoticed for the life of the process; axios sets no + // deadline of its own and does not go through the global fetch wrapper. + timeout: DEFAULT_TIMEOUT_MS, }); export default githubApi; diff --git a/src/config/locales/en-US.json b/src/config/locales/en-US.json index 359da6445c..83665a21bc 100644 --- a/src/config/locales/en-US.json +++ b/src/config/locales/en-US.json @@ -883,6 +883,9 @@ "same_user": "This user already added to list", "fail": "Fail!", "operation_failed_with_reason": "{operation} failed: {reason}", + "request_timed_out": "The connection is taking too long. Check your network and try again.", + "load_failed_retry": "We could not load this. Please try again.", + "retrying": "Retrying...", "wallet_refresh_failed": "Wallet refresh failed. Please try again.", "wallet_refresh_missing_account": "Wallet refresh failed: missing account.", "boost_failed": "Boost failed.", diff --git a/src/config/translationApi.ts b/src/config/translationApi.ts index a33de335fe..84d5213a2f 100644 --- a/src/config/translationApi.ts +++ b/src/config/translationApi.ts @@ -1,10 +1,17 @@ import axios from 'axios'; +import { DEFAULT_TIMEOUT_MS } from '../utils/networkTimeout'; + const translationApi = axios.create({ baseURL: 'https://translate.ecency.com', headers: { 'Content-Type': 'application/json', }, + // Translation is user-initiated and shows its own spinner, so it needs a + // deadline more than most: without one the spinner has no exit. Kept at the + // looser third-party budget rather than the first-party one, because the work + // behind this endpoint legitimately takes longer than a plain API read. + timeout: DEFAULT_TIMEOUT_MS, }); export default translationApi; diff --git a/src/providers/ecency/ePoint.ts b/src/providers/ecency/ePoint.ts index 9b84650e86..5a9316699c 100644 --- a/src/providers/ecency/ePoint.ts +++ b/src/providers/ecency/ePoint.ts @@ -1,5 +1,6 @@ import { getPointsQueryOptions } from '@ecency/sdk'; import { captureException, captureMessage } from '../../utils/sentryUtils'; +import { isAxiosTransportError } from '../../config/axiosTimeout'; import ecencyApi from '../../config/ecencyApi'; import { getQueryClient } from '../queries'; import { EcencyUser, UserPoint } from './ecency.types'; @@ -26,7 +27,14 @@ export const userActivity = async (ty: number, tx = '', bl: string | number = '' return response.data; } catch (error) { console.warn('Failed to push user activity point', error); - captureException(error); + // Transport failures are not reported. The caller retries this mutation and + // then parks it in redux to replay later, so a broken path already recovers + // on its own; reporting each attempt would send three identical events per + // user action for a result the user never sees. Anything the server actually + // answered with is still reported. + if (!isAxiosTransportError(error)) { + captureException(error); + } throw error; } }; diff --git a/src/providers/ecency/ecency.ts b/src/providers/ecency/ecency.ts index fc5467a4f9..7d5f9712bc 100644 --- a/src/providers/ecency/ecency.ts +++ b/src/providers/ecency/ecency.ts @@ -6,6 +6,14 @@ import { SERVER_LIST, withoutBlockedServers } from '../../constants/options/api' import { convertProposalMeta } from './converters'; import { PurchaseRequestData } from './ecency.types'; +/** + * Deliberately far above the instance default. A purchase is not idempotent, so + * the cost of giving up too early (a charge whose outcome nobody knows) is much + * worse than the cost of waiting: this only exists so the call cannot hang for + * the life of the process. + */ +const PURCHASE_ORDER_TIMEOUT_MS = 60000; + /** * ================================================================================ * ECENCY API - MOBILE-SPECIFIC FUNCTIONS @@ -187,7 +195,12 @@ NOTE: data or type PurchaseRequestData should contain body, pass as it is export const purchaseOrder = async (data: PurchaseRequestData) => { try { - const response = await ecencyApi.post('/private-api/purchase-order', data); + // Not idempotent: a client deadline here turns a slow success into an unknown + // outcome, with the user's money on the wrong side of it. Overrides the + // instance default with a ceiling that only trips on a genuinely dead path. + const response = await ecencyApi.post('/private-api/purchase-order', data, { + timeout: PURCHASE_ORDER_TIMEOUT_MS, + }); return response.data; } catch (error) { Sentry.captureException(error); diff --git a/src/providers/plausible/plausible.ts b/src/providers/plausible/plausible.ts index c5afb5e054..ab35b8a8e5 100644 --- a/src/providers/plausible/plausible.ts +++ b/src/providers/plausible/plausible.ts @@ -3,6 +3,9 @@ import axios from 'axios'; import DeviceInfo from 'react-native-device-info'; import * as Sentry from '@sentry/react-native'; +import { isAxiosTransportError } from '../../config/axiosTimeout'; +import { DEFAULT_TIMEOUT_MS } from '../../utils/networkTimeout'; + // Pageview recording only. Post-stats *reads* now go through `@ecency/sdk` // (`getStatsQueryOptions` -> the server-side `/api/stats` proxy), so the stats // API key is no longer shipped in the app — see providers/queries/statsQueries. @@ -15,6 +18,11 @@ const plausibleApi = axios.create({ headers: { 'Content-Type': 'application/json', }, + // Fire-and-forget analytics, which is exactly why it must not hold one of the + // five concurrent slots the platform HTTP client allows per host when the path + // is broken: nothing is waiting on it to notice. Axios sets no deadline of its + // own and does not go through the global fetch wrapper. + timeout: DEFAULT_TIMEOUT_MS, }); export const recordPlausibleEvent = async (urlPath: string, eventName?: string): Promise => { @@ -48,7 +56,15 @@ export const recordPlausibleEvent = async (urlPath: string, eventName?: string): } catch (error) { // Analytics is fire-and-forget: report but do not rethrow, otherwise the // failure surfaces as an unhandled rejection from callers. - Sentry.captureException(error); + // + // Transport failures are NOT reported. This runs once per screen view, and + // now that the request has a deadline every pageview on a broken path + // produces an error instead of a silent hang; across the user base that is + // thousands of identical events a day for a call nothing waits on. Anything + // that is not a transport failure is still reported. + if (!isAxiosTransportError(error)) { + Sentry.captureException(error); + } console.error(`Failed to record event "${eventName}":`, error); } }; diff --git a/src/providers/queries/index.ts b/src/providers/queries/index.ts index b4f16c7d39..86d17b0c63 100644 --- a/src/providers/queries/index.ts +++ b/src/providers/queries/index.ts @@ -1,10 +1,32 @@ -import { Query, QueryClient } from '@tanstack/react-query'; +import { onlineManager, Query, QueryClient } from '@tanstack/react-query'; import { createAsyncStoragePersister } from '@tanstack/query-async-storage-persister'; import AsyncStorage from '@react-native-async-storage/async-storage'; import { PersistQueryClientProviderProps } from '@tanstack/react-query-persist-client'; import { getQueryClient as getQueryClientFromSDK } from '@ecency/sdk'; import VersionNumber from 'react-native-version-number'; +import NetInfo from '@react-native-community/netinfo'; import { initSdkConfig } from './sdk-config'; +import { retryDelay, shouldRetryQuery } from './retryPolicy'; + +/** + * React Query has no connectivity signal of its own in React Native: its + * onlineManager listens for browser `online`/`offline` events, finds none, and + * assumes online forever. Feeding it NetInfo is what makes `refetchOnReconnect` + * work, so a screen left showing an error recovers by itself once the network + * comes back rather than waiting for the user to pull to refresh. + * + * `isInternetReachable` is deliberately only treated as offline when it is + * explicitly false: it is null while the probe is still running, and it can stay + * wrong for a long time on a network that answers the reachability probe but not + * much else. Treating that as offline would be worse than trying and failing. + */ +const _bindOnlineManager = () => { + onlineManager.setEventListener((setOnline) => + NetInfo.addEventListener((state) => { + setOnline(!!state.isConnected && state.isInternetReachable !== false); + }), + ); +}; export const initQueryClient = () => { const asyncStoragePersister = createAsyncStoragePersister({ @@ -16,6 +38,8 @@ export const initQueryClient = () => { throttleTime: 2000, }); + _bindOnlineManager(); + const client = new QueryClient({ defaultOptions: { queries: { @@ -23,6 +47,25 @@ export const initQueryClient = () => { gcTime: 30 * 60 * 1000, // 30 minutes — longer retention for mobile navigation patterns refetchOnWindowFocus: false, refetchOnMount: true, // refetch stale data on screen mount (respects staleTime) + retry: shouldRetryQuery, + retryDelay, + // 'online' would park a query in `paused` whenever the online signal says + // offline, and a paused query is indistinguishable from a loading one in + // the UI: the same indefinite skeleton this work removes. Always attempt + // the request and let it fail visibly; NetInfo's signal is kept for + // refetchOnReconnect only. + networkMode: 'always', + // networkMode 'always' turns refetchOnReconnect OFF by default + // (QueryClient.defaultQueryOptions derives it from networkMode), so it + // has to be asked for explicitly or reconnect recovery is silently lost. + refetchOnReconnect: true, + }, + mutations: { + // Same reasoning: a broadcast or a claim must reach a result the user can + // see, not sit paused behind a connectivity guess. Per-mutation `retry` + // overrides still win. + networkMode: 'always', + retry: false, }, }, }); diff --git a/src/providers/queries/notificationQueries.ts b/src/providers/queries/notificationQueries.ts index 778cd95907..d636cf5142 100644 --- a/src/providers/queries/notificationQueries.ts +++ b/src/providers/queries/notificationQueries.ts @@ -54,6 +54,10 @@ export const useNotificationsQuery = (filter?: NotificationFilters) => { fetchNextPage: infiniteQuery.fetchNextPage, refresh: infiniteQuery.refetch, hasNextPage: infiniteQuery.hasNextPage, + // Only an error when there is nothing to show: a failed "load more" must + // not replace the notifications already on screen. + isError: infiniteQuery.isError && data.length === 0, + error: infiniteQuery.error, }; }; diff --git a/src/providers/queries/postQueries/feedQueries.ts b/src/providers/queries/postQueries/feedQueries.ts index 4fa6925b12..f155d964b5 100644 --- a/src/providers/queries/postQueries/feedQueries.ts +++ b/src/providers/queries/postQueries/feedQueries.ts @@ -127,6 +127,11 @@ export const useFeedQuery = ({ // Avoids new Date() inside select which would defeat TanStack structural sharing. const feedQuery = useInfiniteQuery({ ...(queryOptions as any), + // No client-side retry here. These reads go through hive-tx, which already + // walks the node pool (config.retry, bounded by resilience.totalBudgetFactor) + // before it rejects, so a React Query retry on top only doubles how long the + // screen holds a skeleton before it is allowed to show the error. + retry: false, select: useCallback( (data: any) => { if (!data?.pages) return data; @@ -300,6 +305,11 @@ export const useFeedQuery = ({ fetchNextPage: feedQuery.fetchNextPage, refresh: _refresh, deletePost, + // Surfaced so the list can render a retry instead of a skeleton that never + // resolves. Scoped to the first page: once posts are on screen a failed + // "load more" must not replace them with an error state. + isError: feedQuery.isError && _filteredData.length === 0, + error: feedQuery.error, }; }; diff --git a/src/providers/queries/retryPolicy.test.ts b/src/providers/queries/retryPolicy.test.ts new file mode 100644 index 0000000000..c3fbbc21de --- /dev/null +++ b/src/providers/queries/retryPolicy.test.ts @@ -0,0 +1,54 @@ +import { retryDelay, shouldRetryQuery } from './retryPolicy'; + +const withStatus = (status: number) => ({ response: { status } }); + +describe('shouldRetryQuery', () => { + it('retries a timeout once and then gives up', () => { + const timeout = Object.assign(new Error('Request timed out'), { name: 'TimeoutError' }); + + expect(shouldRetryQuery(0, timeout)).toBe(true); + expect(shouldRetryQuery(1, timeout)).toBe(false); + expect(shouldRetryQuery(2, timeout)).toBe(false); + }); + + it('retries a transport failure with no status once', () => { + expect(shouldRetryQuery(0, new Error('Network request failed'))).toBe(true); + expect(shouldRetryQuery(1, new Error('Network request failed'))).toBe(false); + }); + + it('never retries a query React Query cancelled itself', () => { + // A retry here resurrects work the app deliberately dropped on unmount or on + // a key change, and it is not a failure the user should ever see. + const aborted = Object.assign(new Error('Aborted'), { name: 'AbortError' }); + + expect(shouldRetryQuery(0, aborted)).toBe(false); + }); + + it.each([[400], [401], [403], [404], [422]])( + 'never retries %s, which the server will answer the same way', + (status) => { + expect(shouldRetryQuery(0, withStatus(status))).toBe(false); + }, + ); + + it.each([[408], [425], [429], [500], [502], [503], [504]])( + 'retries %s once, where a second attempt can plausibly help', + (status) => { + expect(shouldRetryQuery(0, withStatus(status))).toBe(true); + expect(shouldRetryQuery(1, withStatus(status))).toBe(false); + }, + ); + + it('reads a status set directly on the error as well as one under response', () => { + expect(shouldRetryQuery(0, { status: 404 })).toBe(false); + expect(shouldRetryQuery(0, { status: 503 })).toBe(true); + }); +}); + +describe('retryDelay', () => { + it('backs off and stays capped', () => { + expect(retryDelay(0)).toBe(1000); + expect(retryDelay(1)).toBe(2000); + expect(retryDelay(10)).toBe(8000); + }); +}); diff --git a/src/providers/queries/retryPolicy.ts b/src/providers/queries/retryPolicy.ts new file mode 100644 index 0000000000..b46ad31500 --- /dev/null +++ b/src/providers/queries/retryPolicy.ts @@ -0,0 +1,52 @@ +// Retry policy for the app-wide QueryClient. +// +// Kept in its own module so the policy can be read and tested without pulling in +// the whole query barrel (AsyncStorage, the SDK, every query hook). + +/** + * Statuses worth a second attempt. Anything else the server actually answered + * with (401, 403, 404, 422, ...) will answer the same way on a retry, so retrying + * only delays the error the screen needs to show. + */ +export const RETRYABLE_STATUS = new Set([408, 425, 429, 500, 502, 503, 504]); + +const statusOf = (error: unknown): number | undefined => { + const candidate = error as { status?: number; response?: { status?: number } }; + return candidate?.status ?? candidate?.response?.status; +}; + +/** + * One retry, and only where a retry can plausibly help. + * + * The point of the policy is that every query reaches a settled state within a + * predictable window. React Query's default is three retries with exponential + * backoff, which on top of a request deadline means a screen could sit on a + * skeleton for minutes before it is allowed to show an error, and it multiplies + * load on exactly the host that is already failing. + * + * The one retry is still worth having: a single wedged connection often clears on + * a fresh one, and aborting the first attempt releases the per-host slot it was + * holding. + */ +export const shouldRetryQuery = (failureCount: number, error: unknown): boolean => { + // React Query cancelled this query itself (unmount, key change, refetch). + // Retrying resurrects work the app deliberately dropped. + if ((error as { name?: string })?.name === 'AbortError') { + return false; + } + + const status = statusOf(error); + if (typeof status === 'number' && !RETRYABLE_STATUS.has(status)) { + return false; + } + + return failureCount < 1; +}; + +/** + * The policy above retries once, so only the first delay is ever used. The cap is + * kept so a per-query `retry` override cannot inherit an unbounded exponential + * backoff. + */ +export const retryDelay = (attemptIndex: number): number => + Math.min(1000 * 2 ** attemptIndex, 8000); diff --git a/src/providers/queries/sdk-config.ts b/src/providers/queries/sdk-config.ts index 3ec103471c..aaea5cf18b 100644 --- a/src/providers/queries/sdk-config.ts +++ b/src/providers/queries/sdk-config.ts @@ -46,23 +46,37 @@ export const initSdkConfig = async (queryClient: QueryClient) => { // Configure image host ConfigManager.setImageHost(Config.NEW_IMAGE_API || 'https://i.ecency.com'); + // Applied before the first await below. Everything after this point depends on + // the network, and this function used to leave the Hive read timeout at the SDK + // default until getNodes() came back, so on exactly the networks the timeout + // exists for it was never installed. + // 10s aligns with checkClient() in providers/hive/hive.ts. With async + // broadcast (broadcast_transaction) the call only awaits mempool accept, + // so 10s is generous headroom while still failing fast on dead nodes. + hiveTxConfig.timeout = 10000; + // Sync saved server preference and fetched nodes to SDK const savedServer = await getServer(); + + const hasValidServer = + typeof savedServer === 'string' && savedServer.trim() !== '' && !isBlockedServer(savedServer); + + // Install the stored preference immediately, before the fetched list is + // awaited: a Hive read issued while getNodes() is still outstanding should go + // to the node the user chose rather than to the SDK's built-in default. + if (hasValidServer) { + ConfigManager.setHiveNodes([savedServer]); + } + // Denied nodes are dropped from BOTH sides. The saved preference is prepended // when it is not already in the fetched list, so filtering only the fetched // list would promote a stored bad node to the front of the pool. const fetchedNodes = withoutBlockedServers(await getNodes()); - const hasValidServer = - typeof savedServer === 'string' && savedServer.trim() !== '' && !isBlockedServer(savedServer); const nodes = hasValidServer && !fetchedNodes.includes(savedServer) ? [savedServer, ...fetchedNodes] : [...fetchedNodes]; ConfigManager.setHiveNodes(nodes); - // 10s aligns with checkClient() in providers/hive/hive.ts. With async - // broadcast (broadcast_transaction) the call only awaits mempool accept, - // so 10s is generous headroom while still failing fast on dead nodes. - hiveTxConfig.timeout = 10000; // Fetch and configure DMCA filters const dmcaLists = await fetchDmcaLists(); diff --git a/src/screens/notification/container/notificationContainer.tsx b/src/screens/notification/container/notificationContainer.tsx index 2be2303179..a6df5583ab 100644 --- a/src/screens/notification/container/notificationContainer.tsx +++ b/src/screens/notification/container/notificationContainer.tsx @@ -172,6 +172,8 @@ const NotificationContainer = ({ navigation }: any) => { globalProps={globalProps} isLoading={selectedQuery.isLoading || selectedQuery.isPending} isFetching={selectedQuery.isFetching} + isError={selectedQuery.isError} + error={selectedQuery.error} /> ); }; diff --git a/src/screens/notification/screen/notificationScreen.tsx b/src/screens/notification/screen/notificationScreen.tsx index 443ec2a0ad..cb20f11605 100644 --- a/src/screens/notification/screen/notificationScreen.tsx +++ b/src/screens/notification/screen/notificationScreen.tsx @@ -19,6 +19,8 @@ const NotificationScreen = ({ isNotificationRefreshing, isLoading, isFetching, + isError, + error, changeSelectedFilter, globalProps, }: any) => { @@ -57,6 +59,8 @@ const NotificationScreen = ({ isNotificationRefreshing={isNotificationRefreshing} isLoading={isLoading} isFetching={isFetching} + isError={isError} + error={error} changeSelectedFilter={changeSelectedFilter} globalProps={globalProps} listRef={notificationsListRef} diff --git a/src/screens/perks/children/questsCard.tsx b/src/screens/perks/children/questsCard.tsx index 171aa86ebd..e65cce169c 100644 --- a/src/screens/perks/children/questsCard.tsx +++ b/src/screens/perks/children/questsCard.tsx @@ -9,7 +9,7 @@ import { useBuyStreakFreeze, } from '@ecency/sdk'; -import { Icon } from '../../../components'; +import { Icon, QueryErrorRetry } from '../../../components'; import { useAuth } from '../../../hooks'; import { useGetQuestsQuery } from '../../../providers/queries/pointQueries'; import RootNavigation from '../../../navigation/rootNavigation'; @@ -32,7 +32,7 @@ const byId = (arr?: { id: string }[]) => Object.fromEntries((arr || []).map((q) const QuestsCard = () => { const intl = useIntl(); const { username, code } = useAuth(); - const { data } = useGetQuestsQuery(username); + const { data, isError, error, isFetching, refetch } = useGetQuestsQuery(username); const [tier, setTier] = useState<(typeof TIERS)[number]>('daily'); const { mutateAsync: buyFreeze, isPending: isBuyingFreeze } = useBuyStreakFreeze(username, code); @@ -94,6 +94,18 @@ const QuestsCard = () => { ); }; + // Without this the card renders every quest at 0/goal when the request fails, + // which is indistinguishable from a user who has done nothing today. Wrong + // progress is worse than no progress, so say so and offer the retry. + if (isError && !data) { + return ( + + {intl.formatMessage({ id: 'perks.quests_title' })} + + + ); + } + return ( {intl.formatMessage({ id: 'perks.quests_title' })} diff --git a/src/utils/abortSignalPolyfill.test.ts b/src/utils/abortSignalPolyfill.test.ts new file mode 100644 index 0000000000..8fe88f19ae --- /dev/null +++ b/src/utils/abortSignalPolyfill.test.ts @@ -0,0 +1,14 @@ +import { createTimeoutReason } from './abortSignalPolyfill'; + +describe('createTimeoutReason', () => { + it('carries TimeoutError as its name, not as its message', () => { + // providers/hive/hive.ts, upvotePopover and @ecency/sdk all branch on + // `err.name === 'TimeoutError'`. An Error whose message happens to read + // 'TimeoutError' has name 'Error' and matches none of them, so a timed-out + // Hive read would be treated as an unknown failure. + const reason = createTimeoutReason(); + + expect(reason.name).toBe('TimeoutError'); + expect(reason).toBeInstanceOf(Error); + }); +}); diff --git a/src/utils/abortSignalPolyfill.ts b/src/utils/abortSignalPolyfill.ts index 4a959427c2..db965d4c9f 100644 --- a/src/utils/abortSignalPolyfill.ts +++ b/src/utils/abortSignalPolyfill.ts @@ -12,12 +12,26 @@ declare global { const AbortSignalRef: any = typeof AbortSignal !== 'undefined' ? AbortSignal : undefined; +/** + * The reason a timed-out signal carries. + * + * `name` is the contract, not `message`: the SDK, providers/hive/hive.ts and + * upvotePopover all branch on `err.name === 'TimeoutError'`, and the SDK builds + * exactly this shape for its own timeouts. An `Error` whose *message* is + * 'TimeoutError' has `name === 'Error'` and matches none of them. + */ +export const createTimeoutReason = (): Error => { + const reason = new Error('The operation was aborted due to timeout'); + reason.name = 'TimeoutError'; + return reason; +}; + if (AbortSignalRef && typeof AbortSignalRef.timeout !== 'function') { AbortSignalRef.timeout = (ms: number): AbortSignal => { const controller: any = new AbortController(); setTimeout(() => { try { - controller.abort(new Error('TimeoutError')); + controller.abort(createTimeoutReason()); } catch { controller.abort(); } @@ -56,5 +70,3 @@ if (AbortSignalRef && typeof AbortSignalRef.any !== 'function') { return controller.signal; }; } - -export {}; diff --git a/src/utils/installFetchDeadline.test.ts b/src/utils/installFetchDeadline.test.ts new file mode 100644 index 0000000000..6cb46a0161 --- /dev/null +++ b/src/utils/installFetchDeadline.test.ts @@ -0,0 +1,67 @@ +/** + * The module installs itself on import, so each case re-imports it inside an + * isolated module registry with a fresh global `fetch` in place. + */ +const loadModule = () => { + let mod: typeof import('./installFetchDeadline'); + jest.isolateModules(() => { + mod = require('./installFetchDeadline'); + }); + return mod!; +}; + +describe('installFetchDeadline', () => { + const originalFetch = globalThis.fetch; + + afterEach(() => { + globalThis.fetch = originalFetch; + jest.useRealTimers(); + }); + + it('replaces the global fetch with a wrapper that bounds the request', async () => { + jest.useFakeTimers(); + const baseFetch = jest.fn( + (_input: any, init?: RequestInit) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + const error = new Error('Aborted'); + error.name = 'AbortError'; + reject(error); + }); + }), + ); + globalThis.fetch = baseFetch as unknown as typeof fetch; + + loadModule(); + expect(globalThis.fetch).not.toBe(baseFetch); + + const promise = globalThis.fetch('https://ecency.com/private-api/x'); + jest.advanceTimersByTime(120000); + + await expect(promise).rejects.toMatchObject({ name: 'TimeoutError' }); + }); + + it('installs once, so a second call cannot stack a second deadline', async () => { + const baseFetch = jest.fn(async () => ({} as Response)); + globalThis.fetch = baseFetch as unknown as typeof fetch; + + const mod = loadModule(); + const afterFirstInstall = globalThis.fetch; + mod.installFetchDeadline(); + + expect(globalThis.fetch).toBe(afterFirstInstall); + + // One layer of wrapping means exactly one call reaches the base fetch. + await globalThis.fetch('https://ecency.com/x'); + expect(baseFetch).toHaveBeenCalledTimes(1); + }); + + it('leaves the global alone when there is no fetch to wrap', () => { + // @ts-expect-error deliberately removing the global for this case + delete globalThis.fetch; + + loadModule(); + + expect(globalThis.fetch).toBeUndefined(); + }); +}); diff --git a/src/utils/installFetchDeadline.ts b/src/utils/installFetchDeadline.ts new file mode 100644 index 0000000000..dfc53e1b7b --- /dev/null +++ b/src/utils/installFetchDeadline.ts @@ -0,0 +1,30 @@ +// Install the fetch deadline on the global, once. +// +// Imported for its side effect from index.js, after ./abortSignalPolyfill and +// before ./App. @ecency/sdk binds `globalThis.fetch` on first use and caches the +// bound reference, so the wrapper has to be in place before the first SDK call. +// +// The guard is a module-level flag, not a marker on `globalThis.fetch`. Sentry's +// instrumentation re-assigns the global at `Sentry.init` time (App.tsx), so a +// marker read off the current global would not survive that and a second install +// would stack a second deadline. + +import { withDeadline } from './networkTimeout'; + +let installed = false; + +export const installFetchDeadline = (): void => { + if (installed) { + return; + } + + const current = globalThis.fetch; + if (typeof current !== 'function') { + return; + } + + installed = true; + globalThis.fetch = withDeadline(current); +}; + +installFetchDeadline(); diff --git a/src/utils/networkTimeout.test.ts b/src/utils/networkTimeout.test.ts new file mode 100644 index 0000000000..3dbe2507a4 --- /dev/null +++ b/src/utils/networkTimeout.test.ts @@ -0,0 +1,245 @@ +import { + DEFAULT_TIMEOUT_MS, + FIRST_PARTY_TIMEOUT_MS, + NO_TIMEOUT, + UPLOAD_TIMEOUT_MS, + hostOf, + resolveTimeoutMs, + withDeadline, +} from './networkTimeout'; + +const okResponse = {} as Response; + +/** Stands in for whatwg-fetch: settles only when the signal it was given aborts. */ +const makeStallingFetch = () => + jest.fn( + (_input: any, init?: RequestInit) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + const error = new Error('Aborted'); + error.name = 'AbortError'; + reject(error); + }); + }), + ); + +/** + * A signal that reports how many listeners are attached, so the wrapper's cleanup + * can be asserted directly rather than inferred. + */ +const makeCountingSignal = () => { + const listeners = new Set<() => void>(); + return { + aborted: false, + addEventListener: (_type: string, fn: () => void) => { + listeners.add(fn); + }, + removeEventListener: (_type: string, fn: () => void) => { + listeners.delete(fn); + }, + listenerCount: () => listeners.size, + }; +}; + +describe('resolveTimeoutMs', () => { + it.each([ + ['a picked video read off disk', 'file:///storage/emulated/0/DCIM/video.mp4'], + ['an Android content URI', 'content://media/external/video/media/42'], + ['an inline data URI', 'data:image/png;base64,AAA'], + ['a blob URL', 'blob:abcd-1234'], + ['a relative path', '/private-api/whatever'], + ])('gives no deadline to %s', (_label, url) => { + expect(resolveTimeoutMs(url)).toBe(NO_TIMEOUT); + }); + + it.each([ + ['https://ecency.com/private-api/x'], + ['https://images.ecency.com/p/abc'], + ['http://ecency.com/dmca/dmca-posts.json'], + ['https://ECENCY.com:443/private-api/x'], + ])('gives our own endpoints the first-party budget: %s', (url) => { + expect(resolveTimeoutMs(url)).toBe(FIRST_PARTY_TIMEOUT_MS); + }); + + it.each([ + // The suffix check must be on '.ecency.com', not on 'ecency.com', or any + // domain merely ending in those characters is treated as ours. + ['https://notecency.com/x'], + ['https://api.coingecko.com/api/v3/simple/price'], + // Userinfo must not be mistaken for the host. + ['https://ecency.com@evil.example/x'], + ])('gives every other host the backstop budget: %s', (url) => { + expect(resolveTimeoutMs(url)).toBe(DEFAULT_TIMEOUT_MS); + }); + + it('gives an upload body the upload ceiling rather than a request budget', () => { + const body = new FormData(); + expect(resolveTimeoutMs('https://ecency.com/private-api/x', { body })).toBe(UPLOAD_TIMEOUT_MS); + }); + + it('does not treat a plain string body as an upload', () => { + expect(resolveTimeoutMs('https://ecency.com/private-api/x', { body: '{"a":1}' })).toBe( + FIRST_PARTY_TIMEOUT_MS, + ); + }); +}); + +describe('hostOf', () => { + it('strips port, userinfo and case', () => { + expect(hostOf('https://user:pw@Images.Ecency.com:8443/p/abc')).toBe('images.ecency.com'); + }); + + it('keeps an IPv6 literal intact', () => { + expect(hostOf('http://[2001:db8::1]:8080/x')).toBe('[2001:db8::1]'); + }); +}); + +describe('withDeadline', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('rejects with a TimeoutError when the deadline expires', async () => { + const baseFetch = makeStallingFetch(); + const wrapped = withDeadline(baseFetch as unknown as typeof fetch, () => 1000); + + const promise = wrapped('https://ecency.com/private-api/thing?code=secret'); + jest.advanceTimersByTime(1000); + + await expect(promise).rejects.toMatchObject({ name: 'TimeoutError' }); + }); + + it('names only the host in the timeout message, never the path or query', async () => { + const baseFetch = makeStallingFetch(); + const wrapped = withDeadline(baseFetch as unknown as typeof fetch, () => 1000); + + const promise = wrapped('https://ecency.com/private-api/thing?code=secret'); + jest.advanceTimersByTime(1000); + + const error = await promise.then( + () => null, + (e) => e as Error, + ); + expect(error?.message).toContain('ecency.com'); + expect(error?.message).not.toContain('private-api'); + expect(error?.message).not.toContain('secret'); + }); + + it("surfaces the caller's own abort rather than relabelling it a timeout", async () => { + const baseFetch = makeStallingFetch(); + const wrapped = withDeadline(baseFetch as unknown as typeof fetch, () => 60000); + const controller = new AbortController(); + + const promise = wrapped('https://ecency.com/x', { signal: controller.signal }); + controller.abort(); + + const error = await promise.then( + () => null, + (e) => e as Error, + ); + expect(error?.name).toBe('AbortError'); + expect(error?.name).not.toBe('TimeoutError'); + }); + + it("keeps the caller's abort when the deadline expires in the same tick", async () => { + // whatwg-fetch defers its abort rejection by a tick, so our own deadline can + // fire in the window between the caller cancelling and the rejection landing. + // Without the `callerSignal.aborted` check the wrapper would relabel a + // deliberate cancellation as a timeout, and the retry policy would retry work + // the app had just dropped. + const baseFetch = jest.fn( + (_input: any, init?: RequestInit) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + setTimeout(() => { + const error = new Error('Aborted'); + error.name = 'AbortError'; + reject(error); + }, 0); + }); + }), + ); + const wrapped = withDeadline(baseFetch as unknown as typeof fetch, () => 1000); + const controller = new AbortController(); + + const promise = wrapped('https://ecency.com/x', { signal: controller.signal }); + controller.abort(); + jest.advanceTimersByTime(1000); + + await expect(promise).rejects.toMatchObject({ name: 'AbortError' }); + }); + + it('still reports a timeout when the caller passed a signal it never aborted', async () => { + const baseFetch = makeStallingFetch(); + const wrapped = withDeadline(baseFetch as unknown as typeof fetch, () => 1000); + const controller = new AbortController(); + + const promise = wrapped('https://ecency.com/x', { signal: controller.signal }); + jest.advanceTimersByTime(1000); + + await expect(promise).rejects.toMatchObject({ name: 'TimeoutError' }); + }); + + it('forwards an already-aborted request untouched, with no deadline of its own', async () => { + const baseFetch = jest.fn((_input: any, _init?: RequestInit) => Promise.resolve(okResponse)); + const wrapped = withDeadline(baseFetch as unknown as typeof fetch, () => 1000); + const controller = new AbortController(); + controller.abort(); + const init: RequestInit = { signal: controller.signal }; + + await wrapped('https://ecency.com/x', init); + + expect(baseFetch.mock.calls[0][1]).toBe(init); + expect(jest.getTimerCount()).toBe(0); + }); + + it('forwards a request that gets no deadline untouched', async () => { + const baseFetch = jest.fn((_input: any, _init?: RequestInit) => Promise.resolve(okResponse)); + const wrapped = withDeadline(baseFetch as unknown as typeof fetch, () => NO_TIMEOUT); + const init: RequestInit = { method: 'GET' }; + + await wrapped('file:///storage/emulated/0/DCIM/video.mp4', init); + + expect(baseFetch.mock.calls[0][1]).toBe(init); + expect(jest.getTimerCount()).toBe(0); + }); + + it('clears the deadline timer once the request succeeds', async () => { + const baseFetch = jest.fn(async () => okResponse); + const wrapped = withDeadline(baseFetch as unknown as typeof fetch, () => 1000); + + const promise = wrapped('https://ecency.com/x'); + expect(jest.getTimerCount()).toBe(1); + + await expect(promise).resolves.toBe(okResponse); + expect(jest.getTimerCount()).toBe(0); + }); + + it('clears the deadline timer once the request fails', async () => { + const baseFetch = jest.fn(async () => { + throw new Error('Network request failed'); + }); + const wrapped = withDeadline(baseFetch as unknown as typeof fetch, () => 1000); + + await expect(wrapped('https://ecency.com/x')).rejects.toThrow('Network request failed'); + expect(jest.getTimerCount()).toBe(0); + }); + + it("detaches its listener from the caller's signal once the request settles", async () => { + const baseFetch = jest.fn(async () => okResponse); + const wrapped = withDeadline(baseFetch as unknown as typeof fetch, () => 1000); + const signal = makeCountingSignal(); + + const promise = wrapped('https://ecency.com/x', { + signal: signal as unknown as AbortSignal, + }); + expect(signal.listenerCount()).toBe(1); + + await promise; + expect(signal.listenerCount()).toBe(0); + }); +}); diff --git a/src/utils/networkTimeout.ts b/src/utils/networkTimeout.ts new file mode 100644 index 0000000000..c211c14276 --- /dev/null +++ b/src/utils/networkTimeout.ts @@ -0,0 +1,261 @@ +// A deadline for every HTTP(S) `fetch` the app makes. +// +// React Native's Android networking stack builds its shared OkHttpClient with +// connect/read/write timeouts of 0, which OkHttp reads as "no timeout" +// (OkHttpClientProvider.createClientBuilder), and the whatwg-fetch polyfill +// never sets `xhr.timeout`. A request that is accepted and then never answered +// therefore stays open for the life of the process: the promise never settles, +// React Query never leaves `pending`, and the screen keeps its skeleton with no +// error and no retry. +// +// OkHttp also dispatches at most 5 concurrent calls PER HOST. Five calls with no +// deadline park every later call to that host in the ready queue indefinitely. +// Bounding each call bounds that queue too: aborting the JS request cancels the +// underlying OkHttp call, which releases the per-host slot. Note this only covers +// `fetch`; an axios client on the same host shares the same five slots and needs +// its own `timeout` (see config/ecencyApi). +// +// Installed on the global `fetch` rather than at each call site because the call +// sites are mostly inside `@ecency/sdk`, which binds `globalThis.fetch` on first +// use and caches the bound reference. +// +// Two deliberate carve-outs: +// * Only http/https get a deadline. `fetch('file://...')` is how a picked video +// is read into a Blob before a resumable upload (providers/speak). That read +// is disk-bound, has no network component, and legitimately runs for minutes +// on a large file. +// * The deadline covers the whole call including the request body, so a short +// one would cut a slow upload mid-send. A binary or multipart body gets the +// same generous ceiling the image upload path already uses. + +/** + * Deadline for our own endpoints, whose latency we actually know: they answer + * well under a second in normal operation. + * + * Not sized on server latency alone. The budget also has to absorb time spent + * queued behind the five-slot-per-host dispatcher during a cold start, and, on a + * host that resolves to several addresses, one dead route failing over to the + * next (bounded by the connect timeout set in MainApplication.kt). 20s is roughly + * twenty times the normal response time, so it can only fire on a path that is + * genuinely not working, which is the point: the users this protects are on bad + * links, and cutting them off early would break the very requests that would + * have succeeded late. + */ +export const FIRST_PARTY_TIMEOUT_MS = 20000; + +/** + * Deadline for every other host. Deliberately looser: third-party APIs and Hive + * RPC nodes carry their own, tighter budgets (hive-tx is configured at 10s in + * providers/queries/sdk-config), so this is only a backstop for a caller that set + * none, and a backstop that fires too early is worse than one that fires late. + */ +export const DEFAULT_TIMEOUT_MS = 30000; + +/** + * Ceiling for a request that carries a body we did not build from a string, i.e. + * an upload. Matches the existing image upload ceiling in config/imageApi. + */ +export const UPLOAD_TIMEOUT_MS = 120000; + +/** Sentinel: this request gets no deadline at all. */ +export const NO_TIMEOUT = 0; + +const FIRST_PARTY_HOST = 'ecency.com'; + +const URL_RE = /^([a-z][a-z0-9+.-]*):\/\/([^/?#]*)/i; + +/** + * Scheme and host of an absolute URL, both lowercased, host without userinfo or + * port. Both are '' for a relative URL. Hand-rolled rather than `new URL()` so + * this module does not depend on the URL polyfill having been installed first. + */ +export const parseUrl = (url: string): { scheme: string; host: string } => { + const match = URL_RE.exec(url); + if (!match) { + return { scheme: '', host: '' }; + } + + const scheme = match[1].toLowerCase(); + const authority = match[2]; + const at = authority.lastIndexOf('@'); + const hostPort = at >= 0 ? authority.slice(at + 1) : authority; + + // IPv6 literals keep their brackets and may not be split on ':'. + if (hostPort.startsWith('[')) { + const end = hostPort.indexOf(']'); + return { scheme, host: (end >= 0 ? hostPort.slice(0, end + 1) : hostPort).toLowerCase() }; + } + + const colon = hostPort.indexOf(':'); + return { scheme, host: (colon >= 0 ? hostPort.slice(0, colon) : hostPort).toLowerCase() }; +}; + +export const hostOf = (url: string): string => parseUrl(url).host; + +/** + * True for a body this module did not get as a string, which in practice means an + * upload: FormData, Blob, ArrayBuffer or a typed-array view. + */ +export const isUploadBody = (body: unknown): boolean => { + if (!body || typeof body === 'string') { + return false; + } + if (typeof FormData !== 'undefined' && body instanceof FormData) { + return true; + } + if (typeof Blob !== 'undefined' && body instanceof Blob) { + return true; + } + if (typeof ArrayBuffer === 'undefined') { + return false; + } + return ArrayBuffer.isView(body) || body instanceof ArrayBuffer; +}; + +export const resolveTimeoutMs = (url: string, init?: RequestInit): number => { + const { scheme } = parseUrl(url); + + // Only network requests get a deadline. `file:`, `content:`, `asset:`, `data:` + // and `blob:` reads are disk- or memory-bound and can legitimately run long. + if (scheme !== 'http' && scheme !== 'https') { + return NO_TIMEOUT; + } + + if (isUploadBody(init?.body)) { + return UPLOAD_TIMEOUT_MS; + } + + const host = hostOf(url); + const isFirstParty = host === FIRST_PARTY_HOST || host.endsWith(`.${FIRST_PARTY_HOST}`); + return isFirstParty ? FIRST_PARTY_TIMEOUT_MS : DEFAULT_TIMEOUT_MS; +}; + +/** + * The error a caller sees when the deadline, rather than the caller, ended the + * request. `name` is the contract: the retry policy in providers/queries reads it, + * the error view picks its message from it, and it keeps a timeout from grouping + * with a user-driven cancellation. + * + * The host, never the URL, so a local file path or a query string cannot reach a + * log or a crash report through the message. + */ +export const createTimeoutError = (url: string, timeoutMs: number): Error => { + const error = new Error(`Request timed out after ${timeoutMs}ms: ${hostOf(url) || 'request'}`); + error.name = 'TimeoutError'; + return error; +}; + +const signalOf = (input: unknown, init?: RequestInit): AbortSignal | undefined => { + // An explicit `signal: null` in init clears the Request's signal per spec, so + // `'signal' in init` is checked before falling back to the Request's own. + if (init && 'signal' in init) { + return init.signal ?? undefined; + } + if (input && typeof input === 'object' && 'signal' in input) { + return (input as { signal?: AbortSignal }).signal ?? undefined; + } + return undefined; +}; + +const urlOf = (input: unknown): string => { + if (typeof input === 'string') { + return input; + } + if (input && typeof input === 'object' && 'url' in input) { + return String((input as { url?: string }).url ?? ''); + } + return ''; +}; + +/** + * Combine the caller's signal with ours, with cleanup. + * + * Hand-rolled rather than `AbortSignal.any` because on this platform + * `AbortSignal` comes from the `abort-controller` package, whose `abort()` takes + * no argument and never populates `signal.reason`. Anything that reads `reason` + * to tell a timeout from a cancellation works under Jest on Node and is dead code + * on device. The wrapper below discriminates on a flag it owns instead. + */ +const combineSignals = ( + caller: AbortSignal | undefined, + ours: AbortSignal, +): { signal: AbortSignal; cleanup: () => void } => { + if (!caller) { + return { signal: ours, cleanup: () => {} }; + } + + const controller = new AbortController(); + const onAbort = () => controller.abort(); + caller.addEventListener('abort', onAbort); + ours.addEventListener('abort', onAbort); + + return { + signal: controller.signal, + cleanup: () => { + caller.removeEventListener('abort', onAbort); + ours.removeEventListener('abort', onAbort); + }, + }; +}; + +/** + * Wrap a fetch implementation so every HTTP(S) request carries a deadline, + * combined with whatever signal the caller already passed. + * + * `resolveTimeout` is a parameter so the deadline policy stays separable from the + * mechanism and a test can exercise expiry without waiting out a real window. + */ +export const withDeadline = ( + baseFetch: typeof fetch, + resolveTimeout: (url: string, init?: RequestInit) => number = resolveTimeoutMs, +): typeof fetch => { + const deadlineFetch = (input: any, init?: RequestInit) => { + const url = urlOf(input); + const timeoutMs = resolveTimeout(url, init); + + if (!timeoutMs || timeoutMs <= 0) { + return baseFetch(input, init); + } + + const callerSignal = signalOf(input, init); + + // Already cancelled: hand it straight through so the rejection is the + // caller's own AbortError and not a timeout we invented. + if (callerSignal?.aborted) { + return baseFetch(input, init); + } + + const controller = new AbortController(); + let expired = false; + const timer = setTimeout(() => { + expired = true; + controller.abort(); + }, timeoutMs); + + const { signal, cleanup } = combineSignals(callerSignal, controller.signal); + + const settle = () => { + clearTimeout(timer); + cleanup(); + }; + + return baseFetch(input, { ...(init ?? {}), signal }).then( + (response) => { + settle(); + return response; + }, + (error) => { + settle(); + // whatwg-fetch rejects every abort with a flat AbortError and this + // platform never carries `signal.reason`, so `expired` is the only + // reliable way to tell our deadline from the caller's cancellation. + if (expired && !callerSignal?.aborted) { + throw createTimeoutError(url, timeoutMs); + } + throw error; + }, + ); + }; + + return deadlineFetch as typeof fetch; +}; From 21971ed287f31de3d66b618a14db022c94f985ed Mon Sep 17 00:00:00 2001 From: feruzm Date: Mon, 31 Aug 2026 13:55:24 +0000 Subject: [PATCH 2/3] fix(network): keep unknown-outcome purchases out of the retry path Review of the deadline work surfaced six ways the new budgets could work against the caller. The Android client no longer sets read and write timeouts. They are idle timeouts applied to every request on the shared client, so 30s of them aborted a purchase that had asked for 60s: the server accepts the order, works on it without sending bytes, and OkHttp gives up first. That is the unknown-outcome payment the wider deadline exists to prevent. Only connectTimeout stays, which is the one that fixes a real hang rather than adding a ceiling. purchaseOrder is no longer resubmitted after a deadline. A timeout says nothing about whether the order landed, so retrying races a request that may still be completing and can register the same receipt twice. The purchase is left unconsumed instead and the recovery path re-attempts it on a later launch, by which time the first attempt has settled and a duplicate is answered with the 409 that path already treats as success. A transport error or a 5xx still retries immediately. urlOf reads href as well as url. A URL object is a valid fetch input and carries its address on href, so reading only url produced no scheme, which resolved to NO_TIMEOUT and left the request unbounded -- the exact failure this work removes. An upload body carried on a Request rather than in init now gets the upload ceiling. fetch(request) keeps the body on the Request, and the polyfill stores the original value privately, so a check that read only init.body handed a slow upload the short budget. Mutations keep React Query's default networkMode. A paused query is a problem because it looks identical to a loading one; a paused mutation is the behaviour we want, held while offline and fired once connectivity returns rather than failing the moment the user taps. --- .../esteem/mobile/android/MainApplication.kt | 20 ++++----- src/containers/inAppPurchaseContainer.tsx | 10 +++++ src/providers/queries/index.ts | 9 ++-- src/utils/installFetchDeadline.test.ts | 1 + src/utils/networkTimeout.test.ts | 35 ++++++++++++++++ src/utils/networkTimeout.ts | 41 ++++++++++++++++--- 6 files changed, 96 insertions(+), 20 deletions(-) diff --git a/android/app/src/main/java/app/esteem/mobile/android/MainApplication.kt b/android/app/src/main/java/app/esteem/mobile/android/MainApplication.kt index 6d4e09b982..bef75511c3 100644 --- a/android/app/src/main/java/app/esteem/mobile/android/MainApplication.kt +++ b/android/app/src/main/java/app/esteem/mobile/android/MainApplication.kt @@ -65,23 +65,23 @@ class MainApplication : Application(), ReactApplication { // real handshake even on a poor link, and low enough that a dead route falls // over to the next one inside the JS deadline (utils/networkTimeout). // - // read and write are idle timeouts, not deadlines: readTimeout bounds the gap - // between bytes and writeTimeout the gap between writes, so a slow but - // progressing upload is unaffected and a large download still completes. - // The per-request deadline stays on callTimeout, which React Native sets per - // request from the JS-side timeout (NetworkingModule). + // read and write are deliberately left alone. They are idle timeouts applied + // to every request on the shared client, so any value low enough to be a + // useful backstop is also low enough to cut short a request that asked for + // longer: a server that accepts an order and then works on it silently would + // be aborted mid-flight, which is the unknown-outcome case the wider + // purchase deadline exists to avoid. The per-request deadline belongs on + // callTimeout, which React Native sets per request from the JS-side timeout + // (NetworkingModule), and every JS caller now carries one. // - // A backstop under the JS-side deadlines, not a replacement for them: it - // catches whatever reaches the network without one. Must be set before the - // first client is created, which happens when NetworkingModule is built. + // Must be set before the first client is created, which happens when + // NetworkingModule is built. OkHttpClientProvider.setOkHttpClientFactory( OkHttpClientFactory { // createClientBuilder(context) keeps React Native's own cookie jar // and its 10MB response cache; only the timeouts change. OkHttpClientProvider.createClientBuilder(this) .connectTimeout(10, TimeUnit.SECONDS) - .readTimeout(30, TimeUnit.SECONDS) - .writeTimeout(30, TimeUnit.SECONDS) .build() } ) diff --git a/src/containers/inAppPurchaseContainer.tsx b/src/containers/inAppPurchaseContainer.tsx index 092535893d..3fd421a361 100644 --- a/src/containers/inAppPurchaseContainer.tsx +++ b/src/containers/inAppPurchaseContainer.tsx @@ -128,6 +128,16 @@ class InAppPurchaseContainer extends Component { if (get(err, 'response.status') === 409) { return undefined; } + // A deadline leaves the server-side outcome unknown: the order may have + // been accepted and still be completing. Resubmitting the same receipt + // here races that in-flight request, so stop and leave the purchase + // unconsumed. The recovery path re-attempts it on a later launch, by + // which time the first attempt has settled and a duplicate is answered + // with the 409 handled above. Only a failure we know did not land -- + // a transport error or a 5xx -- is worth retrying immediately. + if (get(err, 'name') === 'TimeoutError') { + throw err; + } lastErr = err; // Don't sleep after the final attempt -- there is nothing left to retry. if (attempt < PURCHASE_ORDER_MAX_ATTEMPTS - 1) { diff --git a/src/providers/queries/index.ts b/src/providers/queries/index.ts index 86d17b0c63..7e28e7b68f 100644 --- a/src/providers/queries/index.ts +++ b/src/providers/queries/index.ts @@ -61,10 +61,11 @@ export const initQueryClient = () => { refetchOnReconnect: true, }, mutations: { - // Same reasoning: a broadcast or a claim must reach a result the user can - // see, not sit paused behind a connectivity guess. Per-mutation `retry` - // overrides still win. - networkMode: 'always', + // Deliberately NOT 'always'. A paused query is a problem because it looks + // identical to a loading one, but a paused mutation is the behaviour we + // want: it is held while offline and fires once connectivity returns, + // rather than failing in the user's face the moment they tap. Leaving the + // default keeps that. Per-mutation `retry` overrides still win. retry: false, }, }, diff --git a/src/utils/installFetchDeadline.test.ts b/src/utils/installFetchDeadline.test.ts index 6cb46a0161..c55911812c 100644 --- a/src/utils/installFetchDeadline.test.ts +++ b/src/utils/installFetchDeadline.test.ts @@ -5,6 +5,7 @@ const loadModule = () => { let mod: typeof import('./installFetchDeadline'); jest.isolateModules(() => { + // eslint-disable-next-line @typescript-eslint/no-var-requires mod = require('./installFetchDeadline'); }); return mod!; diff --git a/src/utils/networkTimeout.test.ts b/src/utils/networkTimeout.test.ts index 3dbe2507a4..103e49c5c6 100644 --- a/src/utils/networkTimeout.test.ts +++ b/src/utils/networkTimeout.test.ts @@ -82,6 +82,41 @@ describe('resolveTimeoutMs', () => { FIRST_PARTY_TIMEOUT_MS, ); }); + + it('gives the upload ceiling to a body carried on the request rather than in init', () => { + // `fetch(request)` keeps the body on the Request, so a check that only reads + // `init.body` would hand a slow upload the short budget and cut it off. + const request = { url: 'https://ecency.com/private-api/x', body: new FormData() }; + expect(resolveTimeoutMs('https://ecency.com/private-api/x', undefined, request)).toBe( + UPLOAD_TIMEOUT_MS, + ); + }); + + it('gives the upload ceiling to a polyfill request that keeps its body privately', () => { + const request = { url: 'https://ecency.com/private-api/x', _bodyFormData: new FormData() }; + expect(resolveTimeoutMs('https://ecency.com/private-api/x', undefined, request)).toBe( + UPLOAD_TIMEOUT_MS, + ); + }); +}); + +describe('withDeadline with a URL object input', () => { + it('bounds a stalling request made with a URL object rather than a string', async () => { + // A URL object is a valid fetch input and carries its address on `href`, not + // `url`. Reading only `url` yields no scheme, which resolves to NO_TIMEOUT and + // leaves the request pending forever -- the exact failure this module removes. + jest.useFakeTimers(); + try { + const stalling = makeStallingFetch(); + const wrapped = withDeadline(stalling as unknown as typeof fetch); + const pending = wrapped(new URL('https://ecency.com/private-api/x') as any); + const assertion = expect(pending).rejects.toMatchObject({ name: 'TimeoutError' }); + jest.advanceTimersByTime(FIRST_PARTY_TIMEOUT_MS); + await assertion; + } finally { + jest.useRealTimers(); + } + }); }); describe('hostOf', () => { diff --git a/src/utils/networkTimeout.ts b/src/utils/networkTimeout.ts index c211c14276..cffb7921cc 100644 --- a/src/utils/networkTimeout.ts +++ b/src/utils/networkTimeout.ts @@ -112,7 +112,26 @@ export const isUploadBody = (body: unknown): boolean => { return ArrayBuffer.isView(body) || body instanceof ArrayBuffer; }; -export const resolveTimeoutMs = (url: string, init?: RequestInit): number => { +/** + * True when a fetch input carries an upload body. `fetch(request)` keeps the body + * on the Request rather than in `init`, and the polyfill this app uses stores the + * original value on a private field instead of exposing a stream, so both are + * checked. Without this an upload passed that way gets the short deadline. + */ +export const hasUploadInput = (input?: unknown): boolean => { + if (!input || typeof input !== 'object') { + return false; + } + const req = input as Record; + return ( + isUploadBody(req.body) || + isUploadBody(req._bodyFormData) || + isUploadBody(req._bodyBlob) || + isUploadBody(req._bodyArrayBuffer) + ); +}; + +export const resolveTimeoutMs = (url: string, init?: RequestInit, input?: unknown): number => { const { scheme } = parseUrl(url); // Only network requests get a deadline. `file:`, `content:`, `asset:`, `data:` @@ -121,7 +140,7 @@ export const resolveTimeoutMs = (url: string, init?: RequestInit): number => { return NO_TIMEOUT; } - if (isUploadBody(init?.body)) { + if (isUploadBody(init?.body) || hasUploadInput(input)) { return UPLOAD_TIMEOUT_MS; } @@ -161,8 +180,18 @@ const urlOf = (input: unknown): string => { if (typeof input === 'string') { return input; } - if (input && typeof input === 'object' && 'url' in input) { - return String((input as { url?: string }).url ?? ''); + if (input && typeof input === 'object') { + // A Request carries the address on `url`. A URL object carries it on `href` + // and is a valid fetch input in its own right -- fetch stringifies it. Miss + // that and the request resolves to no scheme, which reads as NO_TIMEOUT and + // leaves exactly the unbounded request this module exists to prevent. + const candidate = input as { url?: unknown; href?: unknown }; + if (typeof candidate.url === 'string' && candidate.url) { + return candidate.url; + } + if (typeof candidate.href === 'string') { + return candidate.href; + } } return ''; }; @@ -207,11 +236,11 @@ const combineSignals = ( */ export const withDeadline = ( baseFetch: typeof fetch, - resolveTimeout: (url: string, init?: RequestInit) => number = resolveTimeoutMs, + resolveTimeout: (url: string, init?: RequestInit, input?: unknown) => number = resolveTimeoutMs, ): typeof fetch => { const deadlineFetch = (input: any, init?: RequestInit) => { const url = urlOf(input); - const timeoutMs = resolveTimeout(url, init); + const timeoutMs = resolveTimeout(url, init, input); if (!timeoutMs || timeoutMs <= 0) { return baseFetch(input, init); From 825d8f1c908f2956034032f876790a64d5135c68 Mon Sep 17 00:00:00 2001 From: feruzm Date: Mon, 31 Aug 2026 14:31:02 +0000 Subject: [PATCH 3/3] fix(network): read an unknown connectivity state as online NetInfo reports isConnected as null until the platform has determined a state, which is what the first event after launch carries. Coercing it with !! turned that unknown into offline, while the reachability field beside it was already read as offline only when explicitly false. That gap only became load-bearing once mutations went back to React Query's default network mode: the manager holds a mutation while it says offline and releases it on the next connectivity event, so an unknown state read as offline could park a broadcast or a claim until something else happened to change the network. Attempting the request and letting it fail visibly is recoverable; sitting paused with nothing on screen is not. The rule moves to providers/queries/onlineState so it can be tested without standing up the persister, and both fields are now read the same way: only an explicit false means offline. --- src/providers/queries/index.ts | 10 ++++----- src/providers/queries/onlineState.test.ts | 26 +++++++++++++++++++++++ src/providers/queries/onlineState.ts | 21 ++++++++++++++++++ 3 files changed, 52 insertions(+), 5 deletions(-) create mode 100644 src/providers/queries/onlineState.test.ts create mode 100644 src/providers/queries/onlineState.ts diff --git a/src/providers/queries/index.ts b/src/providers/queries/index.ts index 7e28e7b68f..41ba61ee47 100644 --- a/src/providers/queries/index.ts +++ b/src/providers/queries/index.ts @@ -7,6 +7,7 @@ import VersionNumber from 'react-native-version-number'; import NetInfo from '@react-native-community/netinfo'; import { initSdkConfig } from './sdk-config'; import { retryDelay, shouldRetryQuery } from './retryPolicy'; +import { isOnlineState } from './onlineState'; /** * React Query has no connectivity signal of its own in React Native: its @@ -15,15 +16,14 @@ import { retryDelay, shouldRetryQuery } from './retryPolicy'; * work, so a screen left showing an error recovers by itself once the network * comes back rather than waiting for the user to pull to refresh. * - * `isInternetReachable` is deliberately only treated as offline when it is - * explicitly false: it is null while the probe is still running, and it can stay - * wrong for a long time on a network that answers the reachability probe but not - * much else. Treating that as offline would be worse than trying and failing. + * The online/offline rule itself lives in `./onlineState`, where it can be tested + * without standing up the persister: both NetInfo fields are three-valued and an + * unknown state is read as online rather than offline. */ const _bindOnlineManager = () => { onlineManager.setEventListener((setOnline) => NetInfo.addEventListener((state) => { - setOnline(!!state.isConnected && state.isInternetReachable !== false); + setOnline(isOnlineState(state)); }), ); }; diff --git a/src/providers/queries/onlineState.test.ts b/src/providers/queries/onlineState.test.ts new file mode 100644 index 0000000000..3d0c2ebf33 --- /dev/null +++ b/src/providers/queries/onlineState.test.ts @@ -0,0 +1,26 @@ +import { isOnlineState } from './onlineState'; + +describe('isOnlineState', () => { + it('reports online for a normal connected state', () => { + expect(isOnlineState({ isConnected: true, isInternetReachable: true })).toBe(true); + }); + + it('reports offline only when connectivity is explicitly false', () => { + expect(isOnlineState({ isConnected: false, isInternetReachable: null })).toBe(false); + }); + + it('reports offline when reachability is explicitly false', () => { + expect(isOnlineState({ isConnected: true, isInternetReachable: false })).toBe(false); + }); + + it('treats an unknown connectivity state as online, not offline', () => { + // NetInfo reports isConnected as null until the platform has determined a + // state, which is what the first event after launch carries. Reading that as + // offline parks every mutation until some later connectivity event arrives. + expect(isOnlineState({ isConnected: null, isInternetReachable: null })).toBe(true); + }); + + it('treats a pending reachability probe as online', () => { + expect(isOnlineState({ isConnected: true, isInternetReachable: null })).toBe(true); + }); +}); diff --git a/src/providers/queries/onlineState.ts b/src/providers/queries/onlineState.ts new file mode 100644 index 0000000000..3016436186 --- /dev/null +++ b/src/providers/queries/onlineState.ts @@ -0,0 +1,21 @@ +import type { NetInfoState } from '@react-native-community/netinfo'; + +/** + * Whether NetInfo's view of the network should be reported to React Query as + * online. + * + * Both fields are three-valued, and both are read the same way: only an explicit + * `false` means offline. `isConnected` is null while the platform has not + * determined a state yet, which happens on the very first event after launch, and + * `isInternetReachable` is null while its probe is still running and can stay + * wrong for a long time on a network that answers the probe but little else. + * + * Coercing either unknown to offline is the more damaging mistake. React Query + * holds a mutation while the manager says offline and releases it on the next + * connectivity event, so an unknown state read as offline can park a broadcast or + * a claim until something else happens to change the network. Attempting the + * request and letting it fail visibly is recoverable; sitting paused with nothing + * on screen is not. + */ +export const isOnlineState = (state: Pick) => + state.isConnected !== false && state.isInternetReachable !== false;