Merge pull request #3536 from ecency/fix/network-request-deadlines

fix(network): bound every request and surface the failure

Author: feruzm

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 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.
+        //
+        // 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)
+                    .build()
+            }
+        )
 
         SoLoader.init(this, OpenSourceMergedSoMapping)
 

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';
 

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,
 };

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',
+  },
+});

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<typeof QueryErrorRetry>) => {
+  let tree!: TestRenderer.ReactTestRenderer;
+  TestRenderer.act(() => {
+    tree = TestRenderer.create(<QueryErrorRetry {...props} />);
+  });
+  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();
+  });
+});

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 (
+    <View style={compact ? styles.containerCompact : styles.container}>
+      <Icon
+        iconType="MaterialIcons"
+        name={isTimeout ? 'cloud-off' : 'error-outline'}
+        size={compact ? 18 : 28}
+        style={styles.icon}
+      />
+      <Text style={compact ? styles.messageCompact : styles.message}>
+        {intl.formatMessage({
+          id: isTimeout ? 'alert.request_timed_out' : 'alert.load_failed_retry',
+        })}
+      </Text>
+      <TouchableOpacity
+        style={[styles.button, isRetrying && styles.buttonDisabled]}
+        onPress={() => onRetry()}
+        disabled={isRetrying}
+        accessibilityRole="button"
+      >
+        <Text style={styles.buttonText}>
+          {intl.formatMessage({
+            id: isRetrying ? 'alert.retrying' : 'alert.something_wrong_reload',
+          })}
+        </Text>
+      </TouchableOpacity>
+    </View>
+  );
+};
+
+export default QueryErrorRetry;

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,

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 (
+        <QueryErrorRetry
+          error={error}
+          onRetry={() => getActivities()}
+          isRetrying={isNotificationRefreshing}
+        />
+      );
+    }
+
+    if (isLoading || isFetching || isNotificationRefreshing) {
+      return <ListPlaceHolder />;
+    }
+
+    return (
+      <Text style={globalStyles.hintText}>
+        {intl.formatMessage({ id: 'notification.noactivity' })}
+      </Text>
+    );
+  };
+
   const _renderItem = ({ item }: any) => (
     <NotificationLine
       notification={item}
@@ -141,15 +172,7 @@ const NotificationView = ({
         onEndReachedThreshold={0.3}
         onMomentumScrollBegin={_handleMomentumScrollBegin}
         ListFooterComponent={_renderFooterLoading}
-        ListEmptyComponent={
-          isLoading || isFetching || isNotificationRefreshing ? (
-            <ListPlaceHolder />
-          ) : (
-            <Text style={globalStyles.hintText}>
-              {intl.formatMessage({ id: 'notification.noactivity' })}
-            </Text>
-          )
-        }
+        ListEmptyComponent={_renderEmptyComponent}
         contentContainerStyle={styles.listContentContainer}
         refreshControl={
           <RefreshControl

src/components/tabbedPosts/view/listEmptyView.tsx

@@ -12,7 +12,7 @@ import {
   useCommunitySubscriptionAction,
   useFollowUserAction,
 } from '../../../hooks';
-import { NoPost, PostCardPlaceHolder, UserListItem } from '../..';
+import { NoPost, PostCardPlaceHolder, QueryErrorRetry, UserListItem } from '../..';
 import globalStyles from '../../../globalStyles';
 import { CommunityListItem, EmptyScreen } from '../../basicUIElements';
 import styles from '../styles/tabbedPosts.styles';
@@ -29,9 +29,23 @@ import {
 interface TabEmptyViewProps {
   filterKey: string;
   isNoPost: boolean;
+  /** The first page failed and there is nothing cached to show instead. */
+  isError?: boolean;
+  error?: unknown;
+  isRetrying?: boolean;
+  onRetry?: () => 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 <QueryErrorRetry error={loadError} onRetry={onRetry} isRetrying={isRetrying} />;
+  }
+
   return (
     <View style={styles.placeholderWrapper}>
       <PostCardPlaceHolder />