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

Maybe improve error handling for manifest load/check? #10817

Merged
merged 1 commit into from
Dec 8, 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
13 changes: 3 additions & 10 deletions src/app/bungie-api/bungie-service-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ function dimErrorHandledHttpClient(httpClient: HttpClient): HttpClient {
/**
* if HttpClient throws an error (js, Bungie, http) this enriches it with DIM concepts and then re-throws it
*/
function handleErrors(error: unknown): never {
export function handleErrors(error: unknown): never {
if (error instanceof DOMException && error.name === 'AbortError') {
throw (
navigator.onLine
Expand All @@ -99,6 +99,8 @@ function handleErrors(error: unknown): never {
}

if (error instanceof TypeError) {
// fetch throws this when the user is offline (and a number of other more static cases)
// https://developer.mozilla.org/en-US/docs/Web/API/Window/fetch#exceptions
throw (
navigator.onLine
? new DimError('BungieService.NotConnectedOrBlocked')
Expand All @@ -111,15 +113,6 @@ function handleErrors(error: unknown): never {
}

if (error instanceof HttpStatusError) {
// "I don't think they exist" --Westley, The Princess Bride (1987)
if (error.status === -1) {
throw (
navigator.onLine
? new DimError('BungieService.NotConnectedOrBlocked')
: new DimError('BungieService.NotConnected')
).withError(error);
}

// Token expired and other auth maladies
if (error.status === 401 || error.status === 403) {
throw new DimError('BungieService.NotLoggedIn').withError(error);
Expand Down
30 changes: 11 additions & 19 deletions src/app/manifest/d1-manifest-service.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { handleErrors } from 'app/bungie-api/bungie-service-helper';
import { HttpStatusError, toHttpStatusError } from 'app/bungie-api/http-client';
import { AllD1DestinyManifestComponents } from 'app/destiny1/d1-manifest-types';
import { settingsSelector } from 'app/dim-api/selectors';
import { t } from 'app/i18next-t';
import { loadingEnd, loadingStart } from 'app/shell/actions';
import { del, get, set } from 'app/storage/idb-keyval';
import { ThunkResult } from 'app/store/types';
import { DimError } from 'app/utils/dim-error';
import { convertToError } from 'app/utils/errors';
import { errorLog, infoLog } from 'app/utils/log';
import { dedupePromise } from 'app/utils/promises';
Expand Down Expand Up @@ -43,31 +45,19 @@ function doGetManifest(): ThunkResult<AllD1DestinyManifestComponents> {
}
return manifest;
} catch (err) {
const e = convertToError(err);
let message = e.message;

if (e instanceof TypeError || (e instanceof HttpStatusError && e.status === -1)) {
message = navigator.onLine
? t('BungieService.NotConnectedOrBlocked')
: t('BungieService.NotConnected');
} else if (e instanceof HttpStatusError) {
if (e.status === 503 || e.status === 522 /* cloudflare */) {
message = t('BungieService.Difficulties');
} else if (e.status < 200 || e.status >= 400) {
message = t('BungieService.NetworkError', {
status: e.status,
statusText: e.message,
});
}
let e = convertToError(err);
if (e instanceof DimError && e.cause) {
e = e.cause;
}
if (e.cause instanceof TypeError || e.cause instanceof HttpStatusError) {
} else {
// Something may be wrong with the manifest
deleteManifestFile();
}

const statusText = t('Manifest.Error', { error: message });
errorLog(TAG, 'Manifest loading error', { error: e }, e);
errorLog(TAG, 'Manifest loading error', e);
reportException('manifest load', e);
throw new Error(statusText);
throw new DimError('Manifest.Error', t('Manifest.Error', { error: e.message })).withError(e);
} finally {
dispatch(loadingEnd(t('Manifest.Load')));
}
Expand Down Expand Up @@ -112,6 +102,8 @@ function loadManifestRemote(
saveManifestToIndexedDB(manifest, version);

return manifest;
} catch (e) {
handleErrors(e); // throws
} finally {
dispatch(loadingEnd(t('Manifest.Download')));
}
Expand Down
38 changes: 14 additions & 24 deletions src/app/manifest/manifest-service-json.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { handleErrors } from 'app/bungie-api/bungie-service-helper';
import { HttpStatusError, toHttpStatusError } from 'app/bungie-api/http-client';
import { settingsSelector } from 'app/dim-api/selectors';
import { t } from 'app/i18next-t';
import { loadingEnd, loadingStart } from 'app/shell/actions';
import { del, get, set } from 'app/storage/idb-keyval';
import { ThunkResult } from 'app/store/types';
import { DimError } from 'app/utils/dim-error';
import { emptyArray, emptyObject } from 'app/utils/empty';
import { convertToError, errorMessage } from 'app/utils/errors';
import { convertToError } from 'app/utils/errors';
import { errorLog, infoLog, timer } from 'app/utils/log';
import { dedupePromise } from 'app/utils/promises';
import { LookupTable } from 'app/utils/util-types';
Expand Down Expand Up @@ -152,33 +154,20 @@ function doGetManifest(tableAllowList: string[]): ThunkResult<AllDestinyManifest
throw new Error('Manifest corrupted, please reload');
}
return manifest;
} catch (e) {
let message = errorMessage(e);

if (e instanceof TypeError || (e instanceof HttpStatusError && e.status === -1)) {
message = navigator.onLine
? t('BungieService.NotConnectedOrBlocked')
: t('BungieService.NotConnected');
} else if (e instanceof HttpStatusError) {
if (e.status === 503 || e.status === 522 /* cloudflare */) {
message = t('BungieService.Difficulties');
} else if (e.status < 200 || e.status >= 400) {
message = t('BungieService.NetworkError', {
status: e.status,
statusText: e.message,
});
}
} catch (err) {
let e = convertToError(err);
if (e instanceof DimError && e.cause) {
e = e.cause;
}
if (e.cause instanceof TypeError || e.cause instanceof HttpStatusError) {
} else {
// Something may be wrong with the manifest
await deleteManifestFile();
deleteManifestFile();
}

const statusText = t('Manifest.Error', { error: message });
errorLog(TAG, 'Manifest loading error', e);
reportException('manifest load', e);
const error = new Error(statusText);
error.name = 'ManifestError';
throw error;
throw new DimError('Manifest.Error', t('Manifest.Error', { error: e.message })).withError(e);
} finally {
dispatch(loadingEnd(t('Manifest.Load')));
stopTimer();
Expand Down Expand Up @@ -214,7 +203,8 @@ function loadManifest(tableAllowList: string[]): ThunkResult<AllDestinyManifestC

try {
return await loadManifestFromCache(version, tableAllowList);
} catch {
} catch (e) {
infoLog(TAG, 'Unable to use cached manifest, loading fresh manifest from Bungie.net', e);
return dispatch(loadManifestRemote(version, components, tableAllowList));
}
};
Expand Down Expand Up @@ -286,7 +276,7 @@ export async function downloadManifestComponents(
}
}
if (!body) {
throw error ?? new Error(`Table ${table}`);
handleErrors(error); // throws
}

// I couldn't figure out how to make these types work...
Expand Down
6 changes: 3 additions & 3 deletions src/app/utils/dim-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import { convertToError } from './errors';
* The message is typically a localized error message.
*/
export class DimError extends Error {
// A non-localized string to help identify/categorize errors for DIM developers. Usually the localization key of the message.
/** A non-localized string to help identify/categorize errors for DIM developers. Usually the localization key of the message. */
code?: string;
// The error that caused this error, if there is one. Naming it 'cause' makes it automatically chain in Sentry.
/** The error that caused this error, if there is one. Naming it 'cause' makes it automatically chain in Sentry. */
cause?: Error;
// Whether to show social links in the error report dialog
/** Whether to show social links in the error report dialog. */
showSocials = true;

/** Pass in just a message key to set the message to the localized version of that key, or override with the second parameter. */
Expand Down
17 changes: 8 additions & 9 deletions src/app/wishlists/wishlist-fetch.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { toHttpStatusError } from 'app/bungie-api/http-client';
import { settingsSelector } from 'app/dim-api/selectors';
import { t } from 'app/i18next-t';
import { showNotification } from 'app/notifications/notifications';
Expand Down Expand Up @@ -73,15 +74,13 @@ export function fetchWishList(newWishlistSource?: string): ThunkResult {
let wishListTexts: string[];
try {
wishListTexts = await Promise.all(
wishlistUrlsToFetch.map((url) =>
fetch(url).then((res) => {
if (res.status < 200 || res.status >= 300) {
throw new Error(`failed fetch -- ${res.status} ${res.statusText}`);
}

return res.text();
}),
),
wishlistUrlsToFetch.map(async (url) => {
const res = await fetch(url);
if (res.status < 200 || res.status >= 300) {
throw await toHttpStatusError(res);
}
return res.text();
}),
);

// if this is a new wishlist, set the setting now that we know it's fetchable
Expand Down
Loading