Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(auth): improve error handling #386

Merged
merged 1 commit into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/app-builder/public/locales/en/cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
"case.file.added_date": "Added on",
"case.file.download": "Download",
"case.file.downloading": "Downloading...",
"case.file.errors.downloading_decisions_link": "An unknown error as occured while generating the download link. Please try again later.",
"case.file.errors.downloading_decisions_link.auth_error": "An auth error occured while generating the download link. You may need to log in again.",
"case.file.errors.downloading_decisions_link.unknown": "An unknown error as occured while generating the download link. Please try again later.",
"case.inbox": "Inbox",
"case.inboxes": "Inboxes",
"case.status.open": "open",
Expand Down
3 changes: 3 additions & 0 deletions packages/app-builder/public/locales/en/common.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"validation_error_other": "{{count}} validation errors",
"errors.unknown": "An unknown error as occured",
"errors.account_exists_with_different_credential": "An account already exists with the same email address but different sign-in credentials. Sign in using a provider associated with this email address.",
"errors.popup_blocked_by_client": "The popup has been blocked. Please <EnablePopup>enable popups</EnablePopup> and try again.",
"errors.not_found": "This page could not be found.",
"errors.edit.forbidden_not_draft": "You can only edit a draft version of a scenario.",
"errors.list.duplicate_list_name": "A list with this name already exist",
Expand All @@ -17,6 +18,8 @@
"errors.data.duplicate_table_name": "A table with this name already exist",
"errors.data.duplicate_link_name": "A link with this name already exist",
"errors.add_to_case.invalid": "A decision already belongs to a case",
"errors.firebase_auth_error": "An auth error occured. Try to log in again.",
"errors.firebase_network_error": "A network error occured. Try to log in again.",
"cancel": "Cancel",
"save": "Save",
"delete": "Delete",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"finished_at": "Finished at",
"download_decisions": "Download Decisions",
"downloading_decisions": "Downloading decisions...",
"errors.downloading_decisions_link": "An unknown error as occured while generating the download link. Please try again later.",
"errors.downloading_decisions_link.auth_error": "An auth error occured while generating the download link. You may need to log in again.",
"errors.downloading_decisions_link.unknown": "An unknown error as occured while generating the download link. Please try again later.",
"status_pending": "Pending",
"status_success": "Success",
"status_failure": "Failure"
Expand Down
41 changes: 41 additions & 0 deletions packages/app-builder/src/components/Auth/PopupBlockedError.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { getCurrentBrowser } from '@app-builder/utils/browser';
import { Trans, useTranslation } from 'react-i18next';

export function PopupBlockedError() {
const { t } = useTranslation(['common']);
return (
<div>
<Trans
t={t}
i18nKey="common:errors.popup_blocked_by_client"
components={{
EnablePopup: <EnablePopup />,
}}
/>
</div>
);
}

const hrefMap = {
Safari: 'https://support.apple.com/guide/safari/sfri40696/mac',
Firefox:
'https://support.mozilla.org/en-US/kb/pop-blocker-settings-exceptions-troubleshooting',
Chrome: 'https://support.google.com/chrome/answer/95472',
};

function EnablePopup({ children }: { children?: React.ReactNode }) {
const browser = getCurrentBrowser(navigator.userAgent);
if (browser in hrefMap) {
return (
<a
href={hrefMap[browser as keyof typeof hrefMap]}
target="_blank"
rel="noreferrer noopener"
className="text-purple-100 hover:underline"
>
{children}
</a>
);
}
return <span>{children}</span>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
import {
EmailUnverified,
InvalidLoginCredentials,
NetworkRequestFailed,
useEmailAndPasswordSignIn,
UserNotFoundError,
WrongPasswordError,
Expand Down Expand Up @@ -164,6 +165,8 @@ function ClientSignInWithEmailAndPasswordForm({
setError('credentials', {
message: t('auth:sign_in.errors.invalid_login_credentials'),
});
} else if (error instanceof NetworkRequestFailed) {
toast.error(t('common:errors.firebase_network_error'));
} else {
Sentry.captureException(error);
toast.error(t('common:errors.unknown'));
Expand Down
8 changes: 8 additions & 0 deletions packages/app-builder/src/components/Auth/SignInWithGoogle.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import {
AccountExistsWithDifferentCredential,
NetworkRequestFailed,
PopupBlockedByClient,
useGoogleSignIn,
} from '@app-builder/services/auth/auth.client';
import { type AuthPayload } from '@app-builder/services/auth/auth.server';
Expand All @@ -10,6 +12,8 @@ import { useTranslation } from 'react-i18next';
import { ClientOnly } from 'remix-utils/client-only';
import { Logo } from 'ui-icons';

import { PopupBlockedError } from './PopupBlockedError';

function SignInWithGoogleButton({ onClick }: { onClick?: () => void }) {
const { t } = useTranslation(['auth']);

Expand Down Expand Up @@ -52,6 +56,10 @@ function ClientSignInWithGoogle({
toast.error(
t('common:errors.account_exists_with_different_credential'),
);
} else if (error instanceof PopupBlockedByClient) {
toast.error(<PopupBlockedError />);
} else if (error instanceof NetworkRequestFailed) {
toast.error(t('common:errors.firebase_network_error'));
} else {
Sentry.captureException(error);
toast.error(t('common:errors.unknown'));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import {
AccountExistsWithDifferentCredential,
NetworkRequestFailed,
PopupBlockedByClient,
useMicrosoftSignIn,
} from '@app-builder/services/auth/auth.client';
import { type AuthPayload } from '@app-builder/services/auth/auth.server';
Expand All @@ -10,6 +12,8 @@ import { useTranslation } from 'react-i18next';
import { ClientOnly } from 'remix-utils/client-only';
import { Logo } from 'ui-icons';

import { PopupBlockedError } from './PopupBlockedError';

function SignInWithMicrosoftButton({ onClick }: { onClick?: () => void }) {
const { t } = useTranslation(['auth']);

Expand Down Expand Up @@ -52,6 +56,10 @@ function ClientSignInWithMicrosoft({
toast.error(
t('common:errors.account_exists_with_different_credential'),
);
} else if (error instanceof PopupBlockedByClient) {
toast.error(<PopupBlockedError />);
} else if (error instanceof NetworkRequestFailed) {
toast.error(t('common:errors.firebase_network_error'));
} else {
Sentry.captureException(error);
toast.error(t('common:errors.unknown'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
} from '@app-builder/components/Form';
import {
EmailExistsError,
NetworkRequestFailed,
useEmailAndPasswordSignUp,
WeakPasswordError,
} from '@app-builder/services/auth/auth.client';
Expand Down Expand Up @@ -144,6 +145,8 @@ function ClientSignUpWithEmailAndPasswordForm({
},
{ shouldFocus: true },
);
} else if (error instanceof NetworkRequestFailed) {
toast.error(t('common:errors.firebase_network_error'));
} else {
Sentry.captureException(error);
toast.error(t('common:errors.unknown'));
Expand Down
10 changes: 9 additions & 1 deletion packages/app-builder/src/components/Cases/CaseFiles.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
AlreadyDownloadingError,
AuthRequestError,
useDownloadCaseFiles,
} from '@app-builder/services/DownloadCaseFilesService';
import { formatDateTime, useFormatLanguage } from '@app-builder/utils/format';
Expand Down Expand Up @@ -112,8 +113,15 @@ function FileLink({ caseFileId }: { caseFileId: string }) {
if (e instanceof AlreadyDownloadingError) {
// Already downloading, do nothing
return;
} else if (e instanceof AuthRequestError) {
toast.error(
t('cases:case.file.errors.downloading_decisions_link.auth_error'),
);
} else {
toast.error(
t('cases:case.file.errors.downloading_decisions_link.unknown'),
);
}
toast.error(t('cases:case.file.errors.downloading_decisions_link'));
},
},
);
Expand Down
3 changes: 2 additions & 1 deletion packages/app-builder/src/components/MarbleToaster.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ export function MarbleToaster({
<button
onClick={() => toast.dismiss(currentToast.id)}
aria-label="Close"
className="shrink-0"
>
<Icon icon="cross" className="size-6" />
<Icon icon="cross" className="size-6 shrink-0" />
</button>
) : null}
</>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
AlreadyDownloadingError,
AuthRequestError,
useDownloadDecisions,
} from '@app-builder/services/DownloadDecisionsService';
import { toast } from 'react-hot-toast';
Expand Down Expand Up @@ -37,8 +38,17 @@ function ScheduledExecutionDetailsInternal({
if (e instanceof AlreadyDownloadingError) {
// Already downloading, do nothing
return;
} else if (e instanceof AuthRequestError) {
toast.error(
t(
'scheduledExecution:errors.downloading_decisions_link.auth_error',
),
);
} else {
toast.error(
t('scheduledExecution:errors.downloading_decisions_link.unknown'),
);
}
toast.error(t('scheduledExecution:errors.downloading_decisions_link'));
},
},
);
Expand Down
14 changes: 12 additions & 2 deletions packages/app-builder/src/routes/_builder+/upload+/$objectType.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const UploadForm = ({ objectType }: { objectType: string }) => {
});
const revalidator = useRevalidator();

const { accessToken, backendUrl } = useBackendInfo(
const { getAccessToken, backendUrl } = useBackendInfo(
clientServices.authenticationClientService,
);

Expand Down Expand Up @@ -108,13 +108,23 @@ const UploadForm = ({ objectType }: { objectType: string }) => {
const formData = new FormData();
formData.append('file', file);

const tokenResponse = await getAccessToken();
if (!tokenResponse.success) {
setIsModalOpen(true);
computeModalMessage({
success: false,
errorMessage: t('common:errors.firebase_auth_error'),
});
return;
}

const response = await fetch(
`${backendUrl}/ingestion/${objectType}/batch`,
{
method: 'POST',
body: formData,
headers: {
Authorization: `Bearer ${await accessToken()}`,
Authorization: `Bearer ${tokenResponse.accessToken}`,
},
},
);
Expand Down
18 changes: 10 additions & 8 deletions packages/app-builder/src/routes/ressources+/cases+/upload-file.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ export function UploadFile({ caseDetail }: { caseDetail: CaseDetail }) {
);
}

function toastError(error: string): void {
toast(error, { icon: '❌' });
}

function UploadFileContent({
caseDetail,
setOpen,
Expand All @@ -62,7 +58,7 @@ function UploadFileContent({
const [loading, setLoading] = useState(false);
const revalidator = useRevalidator();

const { accessToken, backendUrl } = useBackendInfo(
const { getAccessToken, backendUrl } = useBackendInfo(
clientServices.authenticationClientService,
);

Expand Down Expand Up @@ -94,6 +90,12 @@ function UploadFileContent({
const file = acceptedFiles[0];
try {
setLoading(true);
const tokenResponse = await getAccessToken();
if (!tokenResponse.success) {
toast.error(t('common:errors.firebase_auth_error'));
return;
}

const formData = new FormData();
formData.append('file', file);

Expand All @@ -103,21 +105,21 @@ function UploadFileContent({
method: 'POST',
body: formData,
headers: {
Authorization: `Bearer ${await accessToken()}`,
Authorization: `Bearer ${tokenResponse.accessToken}`,
},
},
);
if (!response.ok) {
Sentry.captureException(await response.text());
toastError('An error occurred while trying to upload the file.');
toast.error('An error occurred while trying to upload the file.');
return;
}

setLoading(false);
setOpen(false);
} catch (error) {
Sentry.captureException(error);
toastError('An error occurred while trying to upload the file.');
toast.error('An error occurred while trying to upload the file.');
} finally {
setLoading(false);
}
Expand Down
16 changes: 12 additions & 4 deletions packages/app-builder/src/services/DownloadCaseFilesService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import { clientServices } from './init.client';

export class AlreadyDownloadingError extends Error {}
export class FetchLinkError extends Error {}
export class AuthRequestError extends Error {}
type DownloadFileError =
| AlreadyDownloadingError
| FetchLinkError
| DownloadError
| UnknownError;
| UnknownError
| AuthRequestError;

const fileDownloadUrlSchema = z.object({
url: z.string(),
Expand All @@ -23,7 +25,7 @@ export function useDownloadCaseFiles(
{ onError }: { onError?: (error: DownloadFileError) => void } = {},
) {
const [downloading, setDownloading] = useState(false);
const { backendUrl, accessToken } = useBackendInfo(
const { backendUrl, getAccessToken } = useBackendInfo(
clientServices.authenticationClientService,
);

Expand All @@ -36,13 +38,18 @@ export function useDownloadCaseFiles(
}
setDownloading(true);

const tokenResponse = await getAccessToken();
if (!tokenResponse.success) {
throw new AuthRequestError();
}

const downloadLink = `${backendUrl}/cases/files/${encodeURIComponent(
caseFileId,
)}/download_link`;
const response = await fetch(downloadLink, {
method: 'GET',
headers: {
Authorization: `Bearer ${await accessToken()}`,
Authorization: `Bearer ${tokenResponse.accessToken}`,
},
});

Expand All @@ -57,7 +64,8 @@ export function useDownloadCaseFiles(
if (
error instanceof AlreadyDownloadingError ||
error instanceof FetchLinkError ||
error instanceof DownloadError
error instanceof DownloadError ||
error instanceof AuthRequestError
) {
onError?.(error);
} else {
Expand Down
Loading
Loading