kbfs daemon connection status fixes (keybase#22781)

* kbfs daemon connection status fixes

* double space

go/service/simplefs.go

@@ -43,14 +43,43 @@ func (s *SimpleFSHandler) wrapContextWithTimeout(ctx context.Context) (newCtx co
 const waitForTransporterInterval = time.Second / 4
 const waitForTransporterTimeout = time.Minute
 
+type WaitingForKBFSTimeoutError struct {
+	originalError error
+}
+
+// Error implements the error interface.
+func (e WaitingForKBFSTimeoutError) Error() string {
+	return errors.WithMessage(e.originalError, "timeout waiting for kbfs").Error()
+}
+
+// Cause makes it work with errors.Cause().
+func (e WaitingForKBFSTimeoutError) Cause() error {
+	return e.originalError
+}
+
+// HumanError implements the HumanErrorer interface used in libkb/errors.
+// Without this, the Cause will end up being returned to GUI.
+func (e WaitingForKBFSTimeoutError) HumanError() error {
+	return e
+}
+
+// ToStatus implements the ExportableError interface.
+func (e WaitingForKBFSTimeoutError) ToStatus() keybase1.Status {
+	return keybase1.Status{
+		Code: int(keybase1.StatusCode_SCKBFSClientTimeout),
+		Name: "SC_KBFS_CLIENT_TIMEOUT",
+		Desc: "timeout waiting for kbfs",
+	}
+}
+
 func (s *SimpleFSHandler) client(ctx context.Context) (*keybase1.SimpleFSClient, error) {
 	ctx, cancel := context.WithTimeout(ctx, waitForTransporterTimeout)
 	defer cancel()
 	xp := s.G().ConnectionManager.LookupByClientType(keybase1.ClientType_KBFS)
 	for xp == nil {
 		select {
 		case <-ctx.Done():
-			return nil, errors.WithStack(ctx.Err())
+			return nil, WaitingForKBFSTimeoutError{ctx.Err()}
 		default:
 		}
 		time.Sleep(waitForTransporterInterval)

protocol/avdl/keybase1/constants.avdl

@@ -98,6 +98,7 @@ protocol constants {
     SCGenericAPIError_1600,
     SCAPINetworkError_1601,
     SCTimeout_1602,
+    SCKBFSClientTimeout_1603,
     SCProofError_1701,
     SCIdentificationExpired_1702,
     SCSelfNotFound_1703,

shared/actions/fs/index.tsx

@@ -521,6 +521,7 @@ function* pollJournalFlushStatusUntilDone() {
     // eslint-disable-next-line require-atomic-updates
     polling = false
     yield Saga.put(NotificationsGen.createBadgeApp({key: 'kbfsUploading', on: false}))
+    yield Saga.put(FsGen.createCheckKbfsDaemonRpcStatus())
   }
 }
 
@@ -692,28 +693,34 @@ const showMoveOrCopy = () =>
 
 // Can't rely on kbfsDaemonStatus.rpcStatus === 'waiting' as that's set by
 // reducer and happens before this.
-let waitForKbfsDaemonOnFly = false
+let waitForKbfsDaemonInProgress = false
 const waitForKbfsDaemon = async () => {
-  if (waitForKbfsDaemonOnFly) {
+  if (waitForKbfsDaemonInProgress) {
     return
   }
-  waitForKbfsDaemonOnFly = true
+  waitForKbfsDaemonInProgress = true
   try {
-    const connected = await RPCTypes.configWaitForClientRpcPromise({
+    await RPCTypes.configWaitForClientRpcPromise({
       clientType: RPCTypes.ClientType.kbfs,
-      timeout: 20, // 20sec
+      timeout: 60, // 1min. This is arbitrary since we're gonna check again anyway if we're not connected.
     })
-    // eslint-disable-next-line
-    waitForKbfsDaemonOnFly = false
-    return FsGen.createKbfsDaemonRpcStatusChanged({
-      rpcStatus: connected ? Types.KbfsDaemonRpcStatus.Connected : Types.KbfsDaemonRpcStatus.WaitTimeout,
-    })
-  } catch (_) {
-    waitForKbfsDaemonOnFly = false
-    return FsGen.createKbfsDaemonRpcStatusChanged({
-      rpcStatus: Types.KbfsDaemonRpcStatus.WaitTimeout,
-    })
-  }
+  } catch (_) {}
+
+  waitForKbfsDaemonInProgress = false // eslint-disable-line require-atomic-updates
+  return FsGen.createCheckKbfsDaemonRpcStatus()
+}
+
+const checkKbfsDaemonRpcStatus = async (state: Container.TypedState) => {
+  const connected = await RPCTypes.configWaitForClientRpcPromise({
+    clientType: RPCTypes.ClientType.kbfs,
+    timeout: 0, // Don't wait; just check if it's there.
+  })
+  const newStatus = connected ? Types.KbfsDaemonRpcStatus.Connected : Types.KbfsDaemonRpcStatus.Waiting
+  return [
+    state.fs.kbfsDaemonStatus.rpcStatus !== newStatus &&
+      FsGen.createKbfsDaemonRpcStatusChanged({rpcStatus: newStatus}),
+    newStatus === Types.KbfsDaemonRpcStatus.Waiting && FsGen.createWaitForKbfsDaemon(),
+  ]
 }
 
 const startManualCR = async (action: FsGen.StartManualConflictResolutionPayload) => {
@@ -735,7 +742,7 @@ const finishManualCR = async (action: FsGen.FinishManualConflictResolutionPayloa
 // until we get through. After each try we delay for 2s, so this should give us
 // e.g. 12s when n == 6. If it still doesn't work after 12s, something's wrong
 // and we deserve a black bar.
-const checkIfWeReConnectedToMDServerUpToNTimes = async (n: number): Promise<Container.TypedActions> => {
+const checkIfWeReConnectedToMDServerUpToNTimes = async (n: number) => {
   try {
     const onlineStatus = await RPCTypes.SimpleFSSimpleFSGetOnlineStatusRpcPromise()
     return FsGen.createKbfsDaemonOnlineStatusChanged({onlineStatus})
@@ -920,7 +927,7 @@ const loadPathInfo = async (action: FsGen.LoadPathInfoPayload) => {
   })
 }
 
-const loadDownloadInfo = async (action: FsGen.LoadDownloadInfoPayload) => {
+const loadDownloadInfo = async (_: Container.TypedState, action: FsGen.LoadDownloadInfoPayload) => {
   try {
     const res = await RPCTypes.SimpleFSSimpleFSGetDownloadInfoRpcPromise({
       downloadID: action.payload.downloadID,
@@ -934,29 +941,33 @@ const loadDownloadInfo = async (action: FsGen.LoadDownloadInfoPayload) => {
         startTime: res.startTime,
       },
     })
-  } catch {
-    return undefined
+  } catch (error) {
+    return makeUnretriableErrorHandler(action)(error)
   }
 }
 
-const loadDownloadStatus = async () => {
-  const res = await RPCTypes.SimpleFSSimpleFSGetDownloadStatusRpcPromise()
-  return FsGen.createLoadedDownloadStatus({
-    regularDownloads: res.regularDownloadIDs || [],
-    state: new Map(
-      (res.states || []).map(s => [
-        s.downloadID,
-        {
-          canceled: s.canceled,
-          done: s.done,
-          endEstimate: s.endEstimate,
-          error: s.error,
-          localPath: s.localPath,
-          progress: s.progress,
-        },
-      ])
-    ),
-  })
+const loadDownloadStatus = async (_: Container.TypedState, action: FsGen.LoadDownloadStatusPayload) => {
+  try {
+    const res = await RPCTypes.SimpleFSSimpleFSGetDownloadStatusRpcPromise()
+    return FsGen.createLoadedDownloadStatus({
+      regularDownloads: res.regularDownloadIDs || [],
+      state: new Map(
+        (res.states || []).map(s => [
+          s.downloadID,
+          {
+            canceled: s.canceled,
+            done: s.done,
+            endEstimate: s.endEstimate,
+            error: s.error,
+            localPath: s.localPath,
+            progress: s.progress,
+          },
+        ])
+      ),
+    })
+  } catch (error) {
+    return makeUnretriableErrorHandler(action)(error)
+  }
 }
 
 const loadFileContext = async (action: FsGen.LoadFileContextPayload) => {
@@ -1049,9 +1060,10 @@ function* fsSaga() {
   yield* Saga.chainAction2([FsGen.move, FsGen.copy], moveOrCopy)
   yield* Saga.chainAction2([FsGen.showMoveOrCopy, FsGen.showIncomingShare], showMoveOrCopy)
   yield* Saga.chainAction2(
-    [ConfigGen.installerRan, ConfigGen.loggedIn, FsGen.waitForKbfsDaemon, FsGen.userIn],
-    waitForKbfsDaemon
+    [ConfigGen.installerRan, ConfigGen.loggedIn, FsGen.userIn, FsGen.checkKbfsDaemonRpcStatus],
+    checkKbfsDaemonRpcStatus
   )
+  yield* Saga.chainAction2(FsGen.waitForKbfsDaemon, waitForKbfsDaemon)
   yield* Saga.chainAction(FsGen.setTlfSyncConfig, setTlfSyncConfig)
   yield* Saga.chainAction(FsGen.loadTlfSyncConfig, loadTlfSyncConfig)
   yield* Saga.chainAction2([FsGen.getOnlineStatus], getOnlineStatus)
@@ -1074,7 +1086,7 @@ function* fsSaga() {
   yield* Saga.chainAction(FsGen.cancelDownload, cancelDownload)
   yield* Saga.chainAction(FsGen.dismissDownload, dismissDownload)
   yield* Saga.chainAction2(FsGen.loadDownloadStatus, loadDownloadStatus)
-  yield* Saga.chainAction(FsGen.loadDownloadInfo, loadDownloadInfo)
+  yield* Saga.chainAction2(FsGen.loadDownloadInfo, loadDownloadInfo)
 
   yield* Saga.chainAction(FsGen.subscribePath, subscribePath)
   yield* Saga.chainAction(FsGen.subscribeNonPath, subscribeNonPath)

shared/actions/fs/shared.tsx

@@ -15,6 +15,9 @@ const expectedOfflineErrorCodes = [RPCTypes.StatusCode.scapinetworkerror, RPCTyp
 const makeErrorHandler = (action: TypedActions, path: Types.Path | null, retriable: boolean) => (
   error: any
 ): Array<TypedActions> => {
+  if (error?.code === RPCTypes.StatusCode.sckbfsclienttimeout) {
+    return [FsGen.createCheckKbfsDaemonRpcStatus()]
+  }
   const errorDesc = typeof error.desc === 'string' ? error.desc : ''
   if (path && errorDesc) {
     // TODO: KBFS-4143 add and use proper error code for all these
@@ -40,17 +43,6 @@ const makeErrorHandler = (action: TypedActions, path: Types.Path | null, retriab
     ) {
       return [FsGen.createSetPathSoftError({path, softError: Types.SoftError.Nonexistent})]
     }
-    if (errorDesc.includes('KBFS client not found.')) {
-      return [
-        FsGen.createKbfsDaemonRpcStatusChanged({rpcStatus: Types.KbfsDaemonRpcStatus.WaitTimeout}),
-        // We don't retry actions when re-connected, so just route user back
-        // to root in case they get confused by orphan loading state.
-        //
-        // Although this seems impossible to do for nav2 as things are just
-        // pushed on top of each other, so just don't do anything for now.
-        // Perhaps it's OK.
-      ]
-    }
   }
   return [
     FsGen.createFsError({

shared/actions/json/fs.json

@@ -185,6 +185,7 @@
     "setPathItemActionMenuView": {
       "view": "Types.PathItemActionMenuView"
     },
+    "checkKbfsDaemonRpcStatus": {},
     "waitForKbfsDaemon": {},
     "kbfsDaemonRpcStatusChanged": {
       "rpcStatus": "Types.KbfsDaemonRpcStatus"

shared/constants/fs.tsx

@@ -233,7 +233,7 @@ export const defaultDriverStatus: Types.DriverStatus = isLinux
 
 export const unknownKbfsDaemonStatus: Types.KbfsDaemonStatus = {
   onlineStatus: Types.KbfsDaemonOnlineStatus.Unknown,
-  rpcStatus: Types.KbfsDaemonRpcStatus.Unknown,
+  rpcStatus: Types.KbfsDaemonRpcStatus.Waiting,
 }
 
 export const emptySettings: Types.Settings = {

shared/constants/types/fs.tsx

@@ -475,10 +475,8 @@ export type SystemFileManagerIntegration = {
 }
 
 export enum KbfsDaemonRpcStatus {
-  Unknown = 'unknown',
   Connected = 'connected',
   Waiting = 'waiting',
-  WaitTimeout = 'wait-timeout',
 }
 export enum KbfsDaemonOnlineStatus {
   Unknown = 'unknown',

shared/fs/common/hooks.tsx

@@ -16,6 +16,15 @@ import {NavigationEventPayload, SwitchActions} from '@react-navigation/core'
 
 const isPathItem = (path: Types.Path) => Types.getPathLevel(path) > 2 || Constants.hasSpecialFileElement(path)
 
+const noop = () => {}
+const useDispatchWithKbfsDaemonConnectoinGuard = () => {
+  const isConnected = Container.useSelector(
+    state => state.fs.kbfsDaemonStatus.rpcStatus === Types.KbfsDaemonRpcStatus.Connected
+  )
+  const dispatch = Container.useDispatch()
+  return isConnected ? dispatch : noop
+}
+
 const useFsPathSubscriptionEffect = (path: Types.Path, topic: RPCTypes.PathSubscriptionTopic) => {
   const dispatch = Container.useDispatch()
   React.useEffect(() => {
@@ -30,7 +39,7 @@ const useFsPathSubscriptionEffect = (path: Types.Path, topic: RPCTypes.PathSubsc
 }
 
 const useFsNonPathSubscriptionEffect = (topic: RPCTypes.SubscriptionTopic) => {
-  const dispatch = Container.useDispatch()
+  const dispatch = useDispatchWithKbfsDaemonConnectoinGuard()
   React.useEffect(() => {
     const subscriptionID = Constants.makeUUID()
     dispatch(FsGen.createSubscribeNonPath({subscriptionID, topic}))
@@ -97,7 +106,7 @@ export const useFsJournalStatus = () => {
 
 export const useFsOnlineStatus = () => {
   useFsNonPathSubscriptionEffect(RPCTypes.SubscriptionTopic.onlineStatus)
-  const dispatch = Container.useDispatch()
+  const dispatch = useDispatchWithKbfsDaemonConnectoinGuard()
   React.useEffect(() => {
     dispatch(FsGen.createGetOnlineStatus())
   }, [dispatch])