handle api errors in error boundary (#17086)

selfcontained · web-flow · commit 8ddad67d71a2 · 2023-03-31T10:28:40.000+02:00
* handle api errors in error boundary

* handling resetting the error boundary

* splitting out error boundary types
diff --git a/components/dashboard/src/App.tsx b/components/dashboard/src/App.tsx
@@ -4,32 +4,23 @@
  * See License.AGPL.txt in the project root for license information.
  */
 
-import * as GitpodCookie from "@gitpod/gitpod-protocol/lib/util/gitpod-cookie";
 import React, { FC, Suspense } from "react";
 import { AppLoading } from "./app/AppLoading";
 import { AppRoutes } from "./app/AppRoutes";
-import { GitpodErrorBoundary } from "./components/ErrorBoundary";
 import { useCurrentOrg } from "./data/organizations/orgs-query";
 import { useAnalyticsTracking } from "./hooks/use-analytics-tracking";
 import { useUserLoader } from "./hooks/use-user-loader";
 import { Login } from "./Login";
-import { isGitpodIo } from "./utils";
-
-const Setup = React.lazy(() => import(/* webpackPrefetch: true */ "./Setup"));
 
 // Wrap the App in an ErrorBoundary to catch User/Org loading errors
 // This will also catch any errors that happen to bubble all the way up to the top
 const AppWithErrorBoundary: FC = () => {
-    return (
-        <GitpodErrorBoundary>
-            <App />
-        </GitpodErrorBoundary>
-    );
+    return <App />;
 };
 
 // Top level Dashboard App component
 const App: FC = () => {
-    const { user, isSetupRequired, loading } = useUserLoader();
+    const { user, loading } = useUserLoader();
     const currentOrgQuery = useCurrentOrg();
 
     // Setup analytics/tracking
@@ -39,25 +30,7 @@ const App: FC = () => {
         return <AppLoading />;
     }
 
-    // This may be flagged true during user/teams loading
-    if (isSetupRequired) {
-        return (
-            <Suspense fallback={<AppLoading />}>
-                <Setup />
-            </Suspense>
-        );
-    }
-
-    // Redirects to www site if it's the root, no user, and no gp cookie present (has logged in recently)
-    // Should come after the <Setup/> check
-    if (isGitpodIo() && window.location.pathname === "/" && window.location.hash === "" && !user) {
-        // If there's no gp cookie, bounce to www site
-        if (!GitpodCookie.isPresent(document.cookie)) {
-            window.location.href = `https://www.gitpod.io`;
-            return <div></div>;
-        }
-    }
-
+    // Technically this should get handled in the QueryErrorBoundary, but having it here doesn't hurt
     // At this point if there's no user, they should Login
     if (!user) {
         return <Login />;
diff --git a/components/dashboard/src/Login.tsx b/components/dashboard/src/Login.tsx
@@ -6,7 +6,7 @@
 
 import { AuthProviderInfo } from "@gitpod/gitpod-protocol";
 import * as GitpodCookie from "@gitpod/gitpod-protocol/lib/util/gitpod-cookie";
-import { useContext, useEffect, useState, useMemo, useCallback } from "react";
+import { useContext, useEffect, useState, useMemo, useCallback, FC } from "react";
 import { UserContext } from "./user-context";
 import { getGitpodService } from "./service/service";
 import { iconForAuthProvider, openAuthorizeWindow, simplifyProviderName, getSafeURLRedirect } from "./provider-utils";
@@ -47,7 +47,10 @@ export function hasVisitedMarketingWebsiteBefore() {
     return document.cookie.match("gitpod-marketing-website-visited=true");
 }
 
-export function Login() {
+type LoginProps = {
+    onLoggedIn?: () => void;
+};
+export const Login: FC<LoginProps> = ({ onLoggedIn }) => {
     const { setUser } = useContext(UserContext);
 
     const urlHash = useMemo(() => getURLHash(), []);
@@ -96,14 +99,16 @@ export function Login() {
         async (payload?: string) => {
             updateUser().catch(console.error);
 
+            onLoggedIn && onLoggedIn();
+
             // Check for a valid returnTo in payload
             const safeReturnTo = getSafeURLRedirect(payload);
             if (safeReturnTo) {
                 // ... and if it is, redirect to it
                 window.location.replace(safeReturnTo);
             }
         },
-        [updateUser],
+        [onLoggedIn, updateUser],
     );
 
     const openLogin = useCallback(
@@ -261,4 +266,4 @@ export function Login() {
             </div>
         </div>
     );
-}
+};
diff --git a/components/dashboard/src/Setup.tsx b/components/dashboard/src/Setup.tsx
@@ -4,12 +4,16 @@
  * See License.AGPL.txt in the project root for license information.
  */
 
-import { useEffect, useState } from "react";
+import { useCallback, useEffect, useState } from "react";
+import { Button } from "./components/Button";
 import Modal, { ModalBody, ModalFooter, ModalHeader } from "./components/Modal";
 import { getGitpodService, gitpodHostUrl } from "./service/service";
 import { GitIntegrationModal } from "./user-settings/Integrations";
 
-export default function Setup() {
+type Props = {
+    onComplete?: () => void;
+};
+export default function Setup({ onComplete }: Props) {
     const [showModal, setShowModal] = useState<boolean>(false);
 
     useEffect(() => {
@@ -22,16 +26,21 @@ export default function Setup() {
         })();
     }, []);
 
-    const acceptAndContinue = () => {
+    const acceptAndContinue = useCallback(() => {
         setShowModal(true);
-    };
+    }, []);
 
-    const onAuthorize = (payload?: string) => {
-        // run without await, so the integrated closing of new tab isn't blocked
-        (async () => {
-            window.location.href = gitpodHostUrl.asDashboard().toString();
-        })();
-    };
+    const onAuthorize = useCallback(
+        (payload?: string) => {
+            onComplete && onComplete();
+
+            // run without await, so the integrated closing of new tab isn't blocked
+            (async () => {
+                window.location.href = gitpodHostUrl.asDashboard().toString();
+            })();
+        },
+        [onComplete],
+    );
 
     const headerText = "Configure a Git integration with a GitLab, GitHub, or Bitbucket instance.";
 
@@ -61,9 +70,9 @@ export default function Setup() {
                         </div>
                     </ModalBody>
                     <ModalFooter>
-                        <button className={"ml-2"} onClick={() => acceptAndContinue()}>
+                        <Button className={"ml-2"} onClick={acceptAndContinue}>
                             Continue
-                        </button>
+                        </Button>
                     </ModalFooter>
                 </Modal>
             )}
diff --git a/components/dashboard/src/components/error-boundaries/QueryErrorBoundary.tsx b/components/dashboard/src/components/error-boundaries/QueryErrorBoundary.tsx
@@ -0,0 +1,64 @@
+/**
+ * Copyright (c) 2023 Gitpod GmbH. All rights reserved.
+ * Licensed under the GNU Affero General Public License (AGPL).
+ * See License.AGPL.txt in the project root for license information.
+ */
+
+import * as GitpodCookie from "@gitpod/gitpod-protocol/lib/util/gitpod-cookie";
+import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
+import { QueryErrorResetBoundary } from "@tanstack/react-query";
+import { FC, lazy, Suspense } from "react";
+import { ErrorBoundary, FallbackProps } from "react-error-boundary";
+import { AppLoading } from "../../app/AppLoading";
+import { Login } from "../../Login";
+import { isGitpodIo } from "../../utils";
+import { CaughtError } from "./ReloadPageErrorBoundary";
+
+const Setup = lazy(() => import(/* webpackPrefetch: true */ "../../Setup"));
+
+// Error boundary intended to catch and handle expected errors from api calls
+export const QueryErrorBoundary: FC = ({ children }) => {
+    return (
+        <QueryErrorResetBoundary>
+            {({ reset }) => (
+                // Passing reset to onReset so any queries know to retry after boundary is reset
+                <ErrorBoundary FallbackComponent={ExpectedQueryErrorsFallback} onReset={reset}>
+                    {children}
+                </ErrorBoundary>
+            )}
+        </QueryErrorResetBoundary>
+    );
+};
+
+// This handles any expected errors, i.e. errors w/ a code that an api call produced
+const ExpectedQueryErrorsFallback: FC<FallbackProps> = ({ error, resetErrorBoundary }) => {
+    // adjust typing, as we may have caught an api error here w/ a code property
+    const caughtError = error as CaughtError;
+
+    // User needs to Login
+    if (caughtError.code === ErrorCodes.NOT_AUTHENTICATED) {
+        // Before we show a Login screen, check to see if we need to redirect to www site
+        // Redirects if it's the root, no user, and no gp cookie present (has logged in recently)
+        if (isGitpodIo() && window.location.pathname === "/" && window.location.hash === "") {
+            // If there's no gp cookie, bounce to www site
+            if (!GitpodCookie.isPresent(document.cookie)) {
+                window.location.href = `https://www.gitpod.io`;
+                return <div></div>;
+            }
+        }
+
+        return <Login onLoggedIn={resetErrorBoundary} />;
+    }
+
+    // Setup needed before proceeding
+    if (caughtError.code === ErrorCodes.SETUP_REQUIRED) {
+        return (
+            <Suspense fallback={<AppLoading />}>
+                <Setup onComplete={resetErrorBoundary} />
+            </Suspense>
+        );
+    }
+
+    // Otherwise throw the error for default error boundary to catch and handle
+    throw error;
+};
diff --git a/components/dashboard/src/components/error-boundaries/ReloadPageErrorBoundary.tsx b/components/dashboard/src/components/error-boundaries/ReloadPageErrorBoundary.tsx
@@ -4,26 +4,31 @@
  * See License.AGPL.txt in the project root for license information.
  */
 
-import { FC } from "react";
+import { FC, useCallback } from "react";
 import { ErrorBoundary, FallbackProps, ErrorBoundaryProps } from "react-error-boundary";
-import gitpodIcon from "../icons/gitpod.svg";
-import { getGitpodService } from "../service/service";
-import { Heading1, Subheading } from "./typography/headings";
+import gitpodIcon from "../../icons/gitpod.svg";
+import { getGitpodService } from "../../service/service";
+import { Heading1, Subheading } from "../typography/headings";
 
-export const GitpodErrorBoundary: FC = ({ children }) => {
+export type CaughtError = Error & { code?: number };
+
+// Catches any unexpected errors w/ a UI to reload the page. Also reports errors to api
+export const ReloadPageErrorBoundary: FC = ({ children }) => {
     return (
-        <ErrorBoundary FallbackComponent={DefaultErrorFallback} onReset={handleReset} onError={handleError}>
+        <ErrorBoundary FallbackComponent={ReloadPageErrorFallback} onError={handleError}>
             {children}
         </ErrorBoundary>
     );
 };
 
-type CaughtError = Error & { code?: number };
-
-export const DefaultErrorFallback: FC<FallbackProps> = ({ error, resetErrorBoundary }) => {
+export const ReloadPageErrorFallback: FC<Pick<FallbackProps, "error">> = ({ error }) => {
     // adjust typing, as we may have caught an api error here w/ a code property
     const caughtError = error as CaughtError;
 
+    const handleReset = useCallback(() => {
+        window.location.reload();
+    }, []);
+
     const emailSubject = encodeURIComponent("Gitpod Dashboard Error");
     let emailBodyStr = `\n\nError: ${caughtError.message}`;
     if (caughtError.code) {
@@ -43,9 +48,9 @@ export const DefaultErrorFallback: FC<FallbackProps> = ({ error, resetErrorBound
                 .
             </Subheading>
             <div>
-                <button onClick={resetErrorBoundary}>Reload</button>
+                <button onClick={handleReset}>Reload</button>
             </div>
-            <div>
+            <div className="flex flex-col items-center space-y-2">
                 {caughtError.code && (
                     <span>
                         <strong>Code:</strong> {caughtError.code}
@@ -57,10 +62,6 @@ export const DefaultErrorFallback: FC<FallbackProps> = ({ error, resetErrorBound
     );
 };
 
-export const handleReset: ErrorBoundaryProps["onReset"] = () => {
-    window.location.reload();
-};
-
 export const handleError: ErrorBoundaryProps["onError"] = async (error, info) => {
     const url = window.location.toString();
     try {
diff --git a/components/dashboard/src/hooks/use-user-loader.ts b/components/dashboard/src/hooks/use-user-loader.ts
@@ -4,55 +4,38 @@
  * See License.AGPL.txt in the project root for license information.
  */
 
-import { useState, useContext } from "react";
-import { User } from "@gitpod/gitpod-protocol";
+import { useContext } from "react";
 import { UserContext } from "../user-context";
 import { getGitpodService } from "../service/service";
-import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
 import { trackLocation } from "../Analytics";
 import { refreshSearchData } from "../components/RepositoryFinder";
 import { useQuery } from "@tanstack/react-query";
 import { noPersistence } from "../data/setup";
 
 export const useUserLoader = () => {
     const { user, setUser } = useContext(UserContext);
-    const [isSetupRequired, setSetupRequired] = useState(false);
 
     // For now, we're using the user context to store the user, but letting react-query handle the loading
     // In the future, we should remove the user context and use react-query to access the user
     const { isLoading } = useQuery({
         queryKey: noPersistence(["current-user"]),
         queryFn: async () => {
-            let user: User | undefined;
-            try {
-                user = await getGitpodService().server.getLoggedInUser();
-                setUser(user);
-                refreshSearchData();
-            } catch (error) {
-                if (error && "code" in error) {
-                    if (error.code === ErrorCodes.SETUP_REQUIRED) {
-                        setSetupRequired(true);
-                        return;
-                    }
-
-                    // If it was a server error, throw it so we can catch it with an ErrorBoundary
-                    if (error.code >= 500) {
-                        throw error;
-                    }
-
-                    // Other errors will treat user as needing to log in
-                }
-            } finally {
-                trackLocation(!!user);
-            }
+            const user = await getGitpodService().server.getLoggedInUser();
 
             return user || null;
         },
         // We'll let an ErrorBoundary catch the error
         useErrorBoundary: true,
         cacheTime: 1000 * 60 * 60 * 1, // 1 hour
         staleTime: 1000 * 60 * 60 * 1, // 1 hour
+        onSuccess: (loadedUser) => {
+            setUser(loadedUser);
+            refreshSearchData();
+        },
+        onSettled: (loadedUser) => {
+            trackLocation(!!loadedUser);
+        },
     });
 
-    return { user, loading: isLoading, isSetupRequired };
+    return { user, loading: isLoading };
 };
diff --git a/components/dashboard/src/index.tsx b/components/dashboard/src/index.tsx
