Yes that sounds like a good idea. I think we should extract this into a cross-platform hook that:

1. Reduce code duplication here and in D11059
2. Doesn't automatically query both, and instead is able to query conditionally
3. If !usingCommServicesAccessToken || !identitySearchProviderWebsocketConnection, it just queries the keyserver
4. Otherwise, it first tries to query the identity service. If the query fails, it falls back to querying the keyserver

Let's rename this to callLegacyAshoatKeyserverSearchUsers

Let's renamed connected to identitySearchSocketConnected

Let's rename this to searchAshoatKeyserver or something similar

We should still throw if we catch here, so that we dispatch a _FAILED action instead of a _SUCCESS one. You'll need to find another way to call setSearchResults in that case. See my suggestion below

I think we can rewrite this in a cleaner way, while also considering that we should not swallow failures. How about this:

const setSearchResultsFromServer = React.useCallback(
  (userInfos: $ReadOnlyArray<GlobalAccountUserInfo>) => {
    setSearchResults(userInfos.filter(({ id }) => id !== currentUserID));
  },
  [currentUserID],
);

React.useEffect(() => {
  if (forwardLookupSearchText.length === 0) {
    setSearchResults([]);
    return;
  }
  const searchUsersPromise = (async () => {
    if (usingCommServicesAccessToken && identitySearchSocketConnected) {
      try {
        const identitySearchResult = await callIdentitySearchUsers(
          forwardLookupSearchText,
        );
        const userInfos = identitySearchResult.map(user => ({
          id: user.userID,
          username: user.username,
          avatar: null,
        }));
        setSearchResultsFromServer(userInfos);
        return;
      } catch (err) {
        console.error(err);
      }
    }
    const { userInfos: keyserverSearchResult } = await
      callLegacyAshoatKeyserverSearchUsers(
        forwardLookupSearchText,
      );
    setSearchResultsFromServer(keyserverSearchResult);
  })();
  void dispatchActionPromise(searchUsersActionTypes, searchUsersPromise);
}, [
  setSearchResultsFromServer.
  callLegacyAshoatKeyserverSearchUsers,
  callIdentitySearchUsers,
  identitySearchSocketConnected,
  dispatchActionPromise,
  forwardLookupSearchText,
]);

This also lets us skip defining searchServer entirely.

We should always handle the simple case first to reduce the "mental RAM" necessary for the reader

Just clarifying, do we want to throw new Error so the Dispatch can result in an ERROR if this fails or does console.error(err) suffice? Doesn't this stop execution and prevents utilizing the keyserver search as a fallback? My other thought was to set a boolean and throw on a conditional after setSearchResultsFromServer(keyserverSearchResult); runs