Skip to content

Commit

Permalink
improve ignoring fetching removed messages logic, #661
Browse files Browse the repository at this point in the history
  • Loading branch information
vladimiry committed Nov 7, 2024
1 parent e7304b3 commit 29a0aee
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 21 deletions.
20 changes: 16 additions & 4 deletions src/electron-preload/webview/primary/api/db-patch/bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ import * as DatabaseModel from "src/shared/model/database";
import {DbPatchBundle} from "./model";
import {DEFAULT_MESSAGES_STORE_PORTION_SIZE, ONE_SECOND_MS, PACKAGE_VERSION} from "src/shared/const";
import {FsDbAccount, LABEL_TYPE, SYSTEM_FOLDER_IDENTIFIERS} from "src/shared/model/database";
import {isIgnorable404Error} from "../../util";
import {Logger} from "src/shared/model/common";
import {PROTON_MAX_CONCURRENT_FETCH, PROTON_MAX_QUERY_PORTION_LIMIT} from "src/electron-preload/webview/lib/const";
import {ProviderApi} from "src/electron-preload/webview/primary/provider-api/model";
import {resolveCachedConfig, resolveIpcMainApi} from "src/electron-preload/lib/util";
import {resolveCachedConfig, resolveIpcMainApi, sanitizeProtonApiError} from "src/electron-preload/lib/util";
import * as RestModel from "src/electron-preload/webview/lib/rest-model";

export const bootstrapDbPatch = (
Expand Down Expand Up @@ -162,9 +163,20 @@ export const bootstrapDbPatch = (

for (const bootstrappedMessageIdsChunk of chunk(bootstrappedMessageIds, PROTON_MAX_CONCURRENT_FETCH)) {
await Promise.all(bootstrappedMessageIdsChunk.map(async ({ID: storedMessageId}) => {
const {Message} = await providerApi.message.getMessage(storedMessageId);
persistencePortion.push(await Database.buildMail(Message, providerApi));

try {
const {Message} = await providerApi.message.getMessage(storedMessageId);
persistencePortion.push(await Database.buildMail(Message, providerApi));
} catch (error) {
if (!isIgnorable404Error(error)) {
throw error;
}
logger.warn(
`skip message fetching as it has been already removed from the trash before fetch action started`,
JSON.stringify(sanitizeProtonApiError(error)),
);
// skipping "removed" message processing/fetching
return;
}
fetchCount++;
progress$.next({
progress: [
Expand Down
20 changes: 6 additions & 14 deletions src/electron-preload/webview/primary/api/db-patch/patch.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import {curryFunctionMembers} from "src/shared/util";
import * as Database from "src/electron-preload/webview/lib/database-entity";
import {DbPatch} from "src/shared/api/common";
import {isProtonApiError, sanitizeProtonApiError} from "src/electron-preload/lib/util";
import {isIgnorable404Error} from "src/electron-preload/webview/primary/util";
import {LABEL_TYPE, SYSTEM_FOLDER_IDENTIFIERS} from "src/shared/model/database";
import {Logger} from "src/shared/model/common";
import {ProviderApi} from "src/electron-preload/webview/primary/provider-api/model";
import * as RestModel from "src/electron-preload/webview/lib/rest-model";
import {sanitizeProtonApiError} from "src/electron-preload/lib/util";

export const buildDbPatch = async (
providerApi: ProviderApi,
Expand Down Expand Up @@ -103,9 +104,9 @@ export const buildDbPatch = async (
};

if (!nullUpsert) {
// TODO process 404 error of fetching individual entity ("message" case is handled, see "error.data.Code === 15052" check below)
// TODO process 404/422 error of fetching individual entity ("message" case is handled, see "error.data.Code === 15052" check below)
// so we could catch the individual entity fetching error
// 404 error can be ignored as if it occurs because user was moving the email from here to there ending up
// 404/422 error can be ignored as if it occurs because user was moving the email from here to there ending up
// removing it while syncing cycle was in progress

// fetching mails
Expand All @@ -114,19 +115,10 @@ export const buildDbPatch = async (
const response = await providerApi.message.getMessage(id);
patch.mails.upsert.push(await Database.buildMail(response.Message, providerApi));
} catch (error) {
if (
gotTrashed
&& isProtonApiError(error)
&& ( // message has already been removed error condition:
error.status === 422
&& error.data?.Code === 15052
)
) { // ignoring the error as permissible
// TODO figure how to suppress displaying this error on "proton ui" in the case we initiated it (triggered the fetching)
if (gotTrashed && isIgnorable404Error(error)) {
// TODO suppress displaying the error on "proton ui" if message fetching got explicitly triggered vs background sync
logger.warn(
// WARN don't log message-specific data as it might include sensitive fields
`skip message fetching as it has been already removed from the trash before fetch action started`,
// WARN don't log full error as it might include sensitive data
JSON.stringify(sanitizeProtonApiError(error)),
);
} else {
Expand Down
6 changes: 3 additions & 3 deletions src/electron-preload/webview/primary/types.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
export interface ProtonApiError {
config?: unknown;
data?:
& {Error?: unknown; ErrorDescription?: unknown}
& Partial<Pick<import("src/electron-preload/webview/lib/rest-model/response").Response, "Code">>;
data?: {Error?: unknown; ErrorDescription?: unknown; Code?: number; dataCode?: number};
dataCode?: number;
dataError?: string;
message: string;
name: string;
response?: Partial<Response>;
Expand Down
8 changes: 8 additions & 0 deletions src/electron-preload/webview/primary/util.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import {buildDbPatchRetryPipeline, isErrorOnRateLimitedMethodCall} from "src/electron-preload/webview/lib/util";
import {isProtonApiError, sanitizeProtonApiError} from "src/electron-preload/lib/util";

export const isIgnorable404Error: (error: unknown) => boolean = (() => {
const errorCodes: ReadonlyArray<number | undefined> = [2501, 15052];
return (error: unknown): boolean => {
if (!isProtonApiError(error) || error.status !== 422) return false;
return errorCodes.includes(error.data?.Code) || errorCodes.includes(error.dataCode);
};
})();

type preprocessErrorType = Parameters<typeof buildDbPatchRetryPipeline>[0];

export const preprocessError: preprocessErrorType = (() => {
Expand Down

0 comments on commit 29a0aee

Please sign in to comment.