Skip to content

Commit 97ab740

Browse files
authored
Revert "handle api errors in error boundary (#17086)" (#17111)
This reverts commit 8ddad67.
1 parent 500508f commit 97ab740

File tree

7 files changed

+112
-151
lines changed

7 files changed

+112
-151
lines changed

components/dashboard/src/App.tsx

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,23 +4,32 @@
44
* See License.AGPL.txt in the project root for license information.
55
*/
66

7+
import * as GitpodCookie from "@gitpod/gitpod-protocol/lib/util/gitpod-cookie";
78
import React, { FC, Suspense } from "react";
89
import { AppLoading } from "./app/AppLoading";
910
import { AppRoutes } from "./app/AppRoutes";
11+
import { GitpodErrorBoundary } from "./components/ErrorBoundary";
1012
import { useCurrentOrg } from "./data/organizations/orgs-query";
1113
import { useAnalyticsTracking } from "./hooks/use-analytics-tracking";
1214
import { useUserLoader } from "./hooks/use-user-loader";
1315
import { Login } from "./Login";
16+
import { isGitpodIo } from "./utils";
17+
18+
const Setup = React.lazy(() => import(/* webpackPrefetch: true */ "./Setup"));
1419

1520
// Wrap the App in an ErrorBoundary to catch User/Org loading errors
1621
// This will also catch any errors that happen to bubble all the way up to the top
1722
const AppWithErrorBoundary: FC = () => {
18-
return <App />;
23+
return (
24+
<GitpodErrorBoundary>
25+
<App />
26+
</GitpodErrorBoundary>
27+
);
1928
};
2029

2130
// Top level Dashboard App component
2231
const App: FC = () => {
23-
const { user, loading } = useUserLoader();
32+
const { user, isSetupRequired, loading } = useUserLoader();
2433
const currentOrgQuery = useCurrentOrg();
2534

2635
// Setup analytics/tracking
@@ -30,7 +39,25 @@ const App: FC = () => {
3039
return <AppLoading />;
3140
}
3241

33-
// Technically this should get handled in the QueryErrorBoundary, but having it here doesn't hurt
42+
// This may be flagged true during user/teams loading
43+
if (isSetupRequired) {
44+
return (
45+
<Suspense fallback={<AppLoading />}>
46+
<Setup />
47+
</Suspense>
48+
);
49+
}
50+
51+
// Redirects to www site if it's the root, no user, and no gp cookie present (has logged in recently)
52+
// Should come after the <Setup/> check
53+
if (isGitpodIo() && window.location.pathname === "/" && window.location.hash === "" && !user) {
54+
// If there's no gp cookie, bounce to www site
55+
if (!GitpodCookie.isPresent(document.cookie)) {
56+
window.location.href = `https://www.gitpod.io`;
57+
return <div></div>;
58+
}
59+
}
60+
3461
// At this point if there's no user, they should Login
3562
if (!user) {
3663
return <Login />;

components/dashboard/src/Login.tsx

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import { AuthProviderInfo } from "@gitpod/gitpod-protocol";
88
import * as GitpodCookie from "@gitpod/gitpod-protocol/lib/util/gitpod-cookie";
9-
import { useContext, useEffect, useState, useMemo, useCallback, FC } from "react";
9+
import { useContext, useEffect, useState, useMemo, useCallback } from "react";
1010
import { UserContext } from "./user-context";
1111
import { getGitpodService } from "./service/service";
1212
import { iconForAuthProvider, openAuthorizeWindow, simplifyProviderName, getSafeURLRedirect } from "./provider-utils";
@@ -47,10 +47,7 @@ export function hasVisitedMarketingWebsiteBefore() {
4747
return document.cookie.match("gitpod-marketing-website-visited=true");
4848
}
4949

50-
type LoginProps = {
51-
onLoggedIn?: () => void;
52-
};
53-
export const Login: FC<LoginProps> = ({ onLoggedIn }) => {
50+
export function Login() {
5451
const { setUser } = useContext(UserContext);
5552

5653
const urlHash = useMemo(() => getURLHash(), []);
@@ -99,16 +96,14 @@ export const Login: FC<LoginProps> = ({ onLoggedIn }) => {
9996
async (payload?: string) => {
10097
updateUser().catch(console.error);
10198

102-
onLoggedIn && onLoggedIn();
103-
10499
// Check for a valid returnTo in payload
105100
const safeReturnTo = getSafeURLRedirect(payload);
106101
if (safeReturnTo) {
107102
// ... and if it is, redirect to it
108103
window.location.replace(safeReturnTo);
109104
}
110105
},
111-
[onLoggedIn, updateUser],
106+
[updateUser],
112107
);
113108

114109
const openLogin = useCallback(
@@ -266,4 +261,4 @@ export const Login: FC<LoginProps> = ({ onLoggedIn }) => {
266261
</div>
267262
</div>
268263
);
269-
};
264+
}

components/dashboard/src/Setup.tsx

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,12 @@
44
* See License.AGPL.txt in the project root for license information.
55
*/
66

7-
import { useCallback, useEffect, useState } from "react";
8-
import { Button } from "./components/Button";
7+
import { useEffect, useState } from "react";
98
import Modal, { ModalBody, ModalFooter, ModalHeader } from "./components/Modal";
109
import { getGitpodService, gitpodHostUrl } from "./service/service";
1110
import { GitIntegrationModal } from "./user-settings/Integrations";
1211

13-
type Props = {
14-
onComplete?: () => void;
15-
};
16-
export default function Setup({ onComplete }: Props) {
12+
export default function Setup() {
1713
const [showModal, setShowModal] = useState<boolean>(false);
1814

1915
useEffect(() => {
@@ -26,21 +22,16 @@ export default function Setup({ onComplete }: Props) {
2622
})();
2723
}, []);
2824

29-
const acceptAndContinue = useCallback(() => {
25+
const acceptAndContinue = () => {
3026
setShowModal(true);
31-
}, []);
32-
33-
const onAuthorize = useCallback(
34-
(payload?: string) => {
35-
onComplete && onComplete();
27+
};
3628

37-
// run without await, so the integrated closing of new tab isn't blocked
38-
(async () => {
39-
window.location.href = gitpodHostUrl.asDashboard().toString();
40-
})();
41-
},
42-
[onComplete],
43-
);
29+
const onAuthorize = (payload?: string) => {
30+
// run without await, so the integrated closing of new tab isn't blocked
31+
(async () => {
32+
window.location.href = gitpodHostUrl.asDashboard().toString();
33+
})();
34+
};
4435

4536
const headerText = "Configure a Git integration with a GitLab, GitHub, or Bitbucket instance.";
4637

@@ -70,9 +61,9 @@ export default function Setup({ onComplete }: Props) {
7061
</div>
7162
</ModalBody>
7263
<ModalFooter>
73-
<Button className={"ml-2"} onClick={acceptAndContinue}>
64+
<button className={"ml-2"} onClick={() => acceptAndContinue()}>
7465
Continue
75-
</Button>
66+
</button>
7667
</ModalFooter>
7768
</Modal>
7869
)}
Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,26 @@
44
* See License.AGPL.txt in the project root for license information.
55
*/
66

7-
import { FC, useCallback } from "react";
7+
import { FC } from "react";
88
import { ErrorBoundary, FallbackProps, ErrorBoundaryProps } from "react-error-boundary";
9-
import gitpodIcon from "../../icons/gitpod.svg";
10-
import { getGitpodService } from "../../service/service";
11-
import { Heading1, Subheading } from "../typography/headings";
9+
import gitpodIcon from "../icons/gitpod.svg";
10+
import { getGitpodService } from "../service/service";
11+
import { Heading1, Subheading } from "./typography/headings";
1212

13-
export type CaughtError = Error & { code?: number };
14-
15-
// Catches any unexpected errors w/ a UI to reload the page. Also reports errors to api
16-
export const ReloadPageErrorBoundary: FC = ({ children }) => {
13+
export const GitpodErrorBoundary: FC = ({ children }) => {
1714
return (
18-
<ErrorBoundary FallbackComponent={ReloadPageErrorFallback} onError={handleError}>
15+
<ErrorBoundary FallbackComponent={DefaultErrorFallback} onReset={handleReset} onError={handleError}>
1916
{children}
2017
</ErrorBoundary>
2118
);
2219
};
2320

24-
export const ReloadPageErrorFallback: FC<Pick<FallbackProps, "error">> = ({ error }) => {
21+
type CaughtError = Error & { code?: number };
22+
23+
export const DefaultErrorFallback: FC<FallbackProps> = ({ error, resetErrorBoundary }) => {
2524
// adjust typing, as we may have caught an api error here w/ a code property
2625
const caughtError = error as CaughtError;
2726

28-
const handleReset = useCallback(() => {
29-
window.location.reload();
30-
}, []);
31-
3227
const emailSubject = encodeURIComponent("Gitpod Dashboard Error");
3328
let emailBodyStr = `\n\nError: ${caughtError.message}`;
3429
if (caughtError.code) {
@@ -48,9 +43,9 @@ export const ReloadPageErrorFallback: FC<Pick<FallbackProps, "error">> = ({ erro
4843
.
4944
</Subheading>
5045
<div>
51-
<button onClick={handleReset}>Reload</button>
46+
<button onClick={resetErrorBoundary}>Reload</button>
5247
</div>
53-
<div className="flex flex-col items-center space-y-2">
48+
<div>
5449
{caughtError.code && (
5550
<span>
5651
<strong>Code:</strong> {caughtError.code}
@@ -62,6 +57,10 @@ export const ReloadPageErrorFallback: FC<Pick<FallbackProps, "error">> = ({ erro
6257
);
6358
};
6459

60+
export const handleReset: ErrorBoundaryProps["onReset"] = () => {
61+
window.location.reload();
62+
};
63+
6564
export const handleError: ErrorBoundaryProps["onError"] = async (error, info) => {
6665
const url = window.location.toString();
6766
try {

components/dashboard/src/components/error-boundaries/QueryErrorBoundary.tsx

Lines changed: 0 additions & 64 deletions
This file was deleted.

components/dashboard/src/hooks/use-user-loader.ts

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,38 +4,55 @@
44
* See License.AGPL.txt in the project root for license information.
55
*/
66

7-
import { useContext } from "react";
7+
import { useState, useContext } from "react";
8+
import { User } from "@gitpod/gitpod-protocol";
89
import { UserContext } from "../user-context";
910
import { getGitpodService } from "../service/service";
11+
import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
1012
import { trackLocation } from "../Analytics";
1113
import { refreshSearchData } from "../components/RepositoryFinder";
1214
import { useQuery } from "@tanstack/react-query";
1315
import { noPersistence } from "../data/setup";
1416

1517
export const useUserLoader = () => {
1618
const { user, setUser } = useContext(UserContext);
19+
const [isSetupRequired, setSetupRequired] = useState(false);
1720

1821
// For now, we're using the user context to store the user, but letting react-query handle the loading
1922
// In the future, we should remove the user context and use react-query to access the user
2023
const { isLoading } = useQuery({
2124
queryKey: noPersistence(["current-user"]),
2225
queryFn: async () => {
23-
const user = await getGitpodService().server.getLoggedInUser();
26+
let user: User | undefined;
27+
try {
28+
user = await getGitpodService().server.getLoggedInUser();
29+
setUser(user);
30+
refreshSearchData();
31+
} catch (error) {
32+
if (error && "code" in error) {
33+
if (error.code === ErrorCodes.SETUP_REQUIRED) {
34+
setSetupRequired(true);
35+
return;
36+
}
37+
38+
// If it was a server error, throw it so we can catch it with an ErrorBoundary
39+
if (error.code >= 500) {
40+
throw error;
41+
}
42+
43+
// Other errors will treat user as needing to log in
44+
}
45+
} finally {
46+
trackLocation(!!user);
47+
}
2448

2549
return user || null;
2650
},
2751
// We'll let an ErrorBoundary catch the error
2852
useErrorBoundary: true,
2953
cacheTime: 1000 * 60 * 60 * 1, // 1 hour
3054
staleTime: 1000 * 60 * 60 * 1, // 1 hour
31-
onSuccess: (loadedUser) => {
32-
setUser(loadedUser);
33-
refreshSearchData();
34-
},
35-
onSettled: (loadedUser) => {
36-
trackLocation(!!loadedUser);
37-
},
3855
});
3956

40-
return { user, loading: isLoading };
57+
return { user, loading: isLoading, isSetupRequired };
4158
};

0 commit comments

Comments
 (0)