Skip to content

Commit ef9f289

Browse files
committed
handle "429: too many requests" protonmail api error
1 parent ac6d488 commit ef9f289

File tree

11 files changed

+255
-134
lines changed

11 files changed

+255
-134
lines changed

package.json

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -105,17 +105,17 @@
105105
},
106106
"devDependencies": {
107107
"@angular-devkit/build-optimizer": "0.10.6",
108-
"@angular/animations": "7.0.4",
109-
"@angular/common": "7.0.4",
110-
"@angular/compiler": "7.0.4",
111-
"@angular/compiler-cli": "7.0.4",
112-
"@angular/core": "7.0.4",
113-
"@angular/forms": "7.0.4",
114-
"@angular/http": "7.0.4",
115-
"@angular/language-service": "7.0.4",
116-
"@angular/platform-browser": "7.0.4",
117-
"@angular/platform-browser-dynamic": "7.0.4",
118-
"@angular/router": "7.0.4",
108+
"@angular/animations": "7.1.0",
109+
"@angular/common": "7.1.0",
110+
"@angular/compiler": "7.1.0",
111+
"@angular/compiler-cli": "7.1.0",
112+
"@angular/core": "7.1.0",
113+
"@angular/forms": "7.1.0",
114+
"@angular/http": "7.1.0",
115+
"@angular/language-service": "7.1.0",
116+
"@angular/platform-browser": "7.1.0",
117+
"@angular/platform-browser-dynamic": "7.1.0",
118+
"@angular/router": "7.1.0",
119119
"@angularclass/hmr": "2.1.3",
120120
"@email-securely-app/import-sort-style": "0.1.0",
121121
"@ng-select/ng-select": "2.12.1",
@@ -132,11 +132,12 @@
132132
"@types/keytar": "4.0.1",
133133
"@types/mini-css-extract-plugin": "0.2.0",
134134
"@types/mkdirp": "0.5.2",
135-
"@types/node": "10.12.9",
135+
"@types/node": "10.12.10",
136136
"@types/node-fetch": "2.1.4",
137137
"@types/p-queue": "3.0.0",
138138
"@types/ramda": "0.26.0",
139139
"@types/randomstring": "1.1.6",
140+
"@types/rolling-rate-limiter": "0.1.0",
140141
"@types/sanitize-html": "1.18.2",
141142
"@types/semver": "5.5.0",
142143
"@types/sinon": "5.0.7",
@@ -187,11 +188,11 @@
187188
"karma-webpack": "4.0.0-rc.2",
188189
"keysim": "2.1.0",
189190
"less-loader": "4.1.0",
190-
"lint-staged": "8.0.5",
191-
"mini-css-extract-plugin": "0.4.4",
191+
"lint-staged": "8.1.0",
192+
"mini-css-extract-plugin": "0.4.5",
192193
"mkdirp": "0.5.1",
193194
"ng2-dragula": "2.1.1",
194-
"ngx-bootstrap": "3.1.1",
195+
"ngx-bootstrap": "3.1.2",
195196
"node-fetch": "2.3.0",
196197
"node-sass": "4.10.0",
197198
"npm-run-all": "4.1.3",

src/@types/rolling-rate-limiter/index.d.ts

Lines changed: 0 additions & 1 deletion
This file was deleted.

src/electron-main/constants.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {Model as StoreModel} from "fs-json-store";
55
import {randomBytes} from "crypto";
66

77
import {Config, ENCRYPTION_DERIVATION_PRESETS, KEY_DERIVATION_PRESETS, Settings} from "src/shared/model/options";
8+
import {ONE_SECOND_MS} from "src/shared/constants";
89

910
export const INITIAL_STORES: Readonly<{
1011
config: () => Config;
@@ -28,6 +29,14 @@ export const INITIAL_STORES: Readonly<{
2829
window: {
2930
bounds: {width: 1024, height: 768},
3031
},
32+
fetchingRateLimiting: { // 250 requests in 60 seconds
33+
intervalMs: ONE_SECOND_MS * 60,
34+
maxInInterval: 250,
35+
},
36+
timeouts: {
37+
// "fetchingRateLimiting" values need to be taking into the account defining the "fetching" timeout
38+
fetching: ONE_SECOND_MS * 60 * 10, // 10 minutes
39+
},
3140
};
3241
},
3342
settings: () => {

src/electron-main/storage-upgrade.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@ const CONFIG_UPGRADES: Record<string, (config: Config) => void> = {
1616
config.logLevel = INITIAL_STORES.config().logLevel;
1717
}
1818
},
19+
"2.0.0-beta.7": (config) => {
20+
if (typeof config.fetchingRateLimiting === "undefined") {
21+
config.fetchingRateLimiting = INITIAL_STORES.config().fetchingRateLimiting;
22+
}
23+
if (typeof config.timeouts === "undefined") {
24+
config.timeouts = INITIAL_STORES.config().timeouts;
25+
}
26+
},
1927
};
2028

2129
const SETTINGS_UPGRADES: Record<string, (settings: Settings) => void> = {

src/electron-preload/webview/protonmail/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ async function bootstrapDbPatch(): Promise<Unpacked<ReturnType<ProtonmailApi["bu
424424
}
425425
const [messages, contacts, labels] = await Promise.all([
426426
// messages
427-
(async (query = {Page: 0, PageSize: 150}, throttleOptions = {maxInProgress: 3, failFast: true}) => {
427+
(async (query = {Page: 0, PageSize: 150}, throttleOptions = {maxInProgress: 1, failFast: true}) => {
428428
type Response = Unpacked<ReturnType<typeof api.conversation.query>>;
429429
const responseItems: Response["data"]["Conversations"] = [];
430430
let response: Response | undefined;

src/electron-preload/webview/protonmail/lib/api.ts

Lines changed: 115 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
1+
import PQueue from "p-queue";
2+
import rateLimiter from "rolling-rate-limiter";
3+
import {v4 as uuid} from "uuid";
4+
15
import * as Rest from "./rest";
6+
import {IPC_MAIN_API} from "src/shared/api/main";
27
import {Unpacked} from "src/shared/types";
38
import {WEBVIEW_LOGGERS} from "src/electron-preload/webview/constants";
4-
import {curryFunctionMembers} from "src/shared/util";
9+
import {asyncDelay, curryFunctionMembers} from "src/shared/util";
510

611
// TODO consider executing direct $http calls
712
// in order to not depend on Protonmail WebClient's AngularJS factories/services
@@ -10,6 +15,9 @@ export interface Api {
1015
url: {
1116
build: (module: string) => () => string;
1217
};
18+
lazyLoader: {
19+
app: () => Promise<void>;
20+
};
1321
messageModel: (message: Rest.Model.Message) => {
1422
clearTextBody: () => Promise<string>;
1523
};
@@ -43,13 +51,37 @@ export interface Api {
4351
}
4452

4553
const logger = curryFunctionMembers(WEBVIEW_LOGGERS.protonmail, "[lib/api]");
54+
const resolveServiceLogger = curryFunctionMembers(logger, "resolveService()");
55+
const rateLimitedApiCallingQueue: PQueue<PQueue.DefaultAddOptions> = new PQueue({concurrency: 1});
56+
const ipcMainApiClient = IPC_MAIN_API.buildClient();
4657
const state: { api?: Promise<Api> } = {};
4758

59+
let rateLimitedMethodsCallCount = 0;
60+
4861
export async function resolveApi(): Promise<Api> {
62+
logger.info(`resolveApi()`);
4963
if (state.api) {
5064
return state.api;
5165
}
5266

67+
const rateLimiting = {
68+
rateLimiterTick: await (async () => {
69+
const {fetchingRateLimiting: config} = await ipcMainApiClient("readConfig")().toPromise();
70+
logger.debug(JSON.stringify({
71+
fetchingRateLimiter_111: {
72+
interval: config.intervalMs,
73+
maxInInterval: config.maxInInterval,
74+
},
75+
}));
76+
const limiter = rateLimiter({
77+
interval: config.intervalMs,
78+
maxInInterval: config.maxInInterval,
79+
});
80+
const key = `webview:protonmail-api:${uuid()}`;
81+
return () => limiter(key);
82+
})(),
83+
};
84+
5385
return state.api = (async () => {
5486
const angular: angular.IAngularStatic | undefined = (window as any).angular;
5587
const injector = angular && angular.element(document.body).injector();
@@ -58,34 +90,99 @@ export async function resolveApi(): Promise<Api> {
5890
throw new Error(`Failed to resolve "injector" variable`);
5991
}
6092

61-
await injectorGet<{ app: () => Promise<void> }>(injector, "lazyLoader").app();
93+
const lazyLoader = resolveService<Api["lazyLoader"]>(injector, "lazyLoader");
94+
95+
await lazyLoader.app();
6296

6397
// TODO validate types of all the described constants/functions in a declarative way
6498
// so app gets protonmail breaking changes noticed on early stage
6599

66100
return {
67-
$http: injectorGet<Api["$http"]>(injector, "$http"),
68-
url: injectorGet<Api["url"]>(injector, "url"),
69-
messageModel: injectorGet<Api["messageModel"]>(injector, "messageModel"),
70-
conversation: injectorGet<Api["conversation"]>(injector, "conversationApi"),
71-
message: injectorGet<Api["message"]>(injector, "messageApi"),
72-
contact: injectorGet<Api["contact"]>(injector, "Contact"),
73-
label: injectorGet<Api["label"]>(injector, "Label"),
74-
events: injectorGet<Api["events"]>(injector, "Events"),
75-
vcard: injectorGet<Api["vcard"]>(injector, "vcard"),
101+
lazyLoader,
102+
$http: resolveService<Api["$http"]>(injector, "$http"),
103+
url: resolveService<Api["url"]>(injector, "url"),
104+
messageModel: resolveService<Api["messageModel"]>(injector, "messageModel"),
105+
conversation: resolveService<Api["conversation"]>(injector, "conversationApi", {...rateLimiting, methods: ["get", "query"]}),
106+
message: resolveService<Api["message"]>(injector, "messageApi", {...rateLimiting, methods: ["get", "query"]}),
107+
contact: resolveService<Api["contact"]>(injector, "Contact", {...rateLimiting, methods: ["get", "all"]}),
108+
label: resolveService<Api["label"]>(injector, "Label", {...rateLimiting, methods: ["query"]}),
109+
events: resolveService<Api["events"]>(injector, "Events", {...rateLimiting, methods: ["get", "getLatestID"]}),
110+
vcard: resolveService<Api["vcard"]>(injector, "vcard"),
76111
};
77112
})();
78113
}
79114

80-
function injectorGet<T>(injector: ng.auto.IInjectorService, name: string): T {
81-
logger.info(`injectorGet()`);
82-
const result = injector.get<T | undefined>(name);
115+
type KeepAsyncFunctionsProps<T> = {
116+
[K in keyof T]: T[K] extends (args: any) => Promise<infer U> ? T[K] : never
117+
};
118+
119+
function resolveService<T extends Api[keyof Api]>(
120+
injector: ng.auto.IInjectorService,
121+
serviceName: string,
122+
rateLimiting?: {
123+
rateLimiterTick: () => number;
124+
methods: Array<keyof KeepAsyncFunctionsProps<T>>,
125+
},
126+
): T {
127+
resolveServiceLogger.info();
128+
const service = injector.get<T | undefined>(serviceName);
129+
130+
if (!service) {
131+
throw new Error(`Failed to resolve "${serviceName}" service`);
132+
}
133+
134+
resolveServiceLogger.verbose(`"${serviceName}" keys`, JSON.stringify(Object.keys(service)));
83135

84-
if (!result) {
85-
throw new Error(`Failed to resolve "${name}" service`);
136+
if (!rateLimiting) {
137+
return service;
86138
}
87139

88-
logger.verbose(`injectorGet()`, `"${name}" keys`, JSON.stringify(Object.keys(result)));
140+
const clonedService = {...service} as T;
141+
142+
for (const method of rateLimiting.methods) {
143+
const originalMethod = clonedService[method];
144+
const _fullMethodName = `${serviceName}.${method}`;
145+
146+
if (typeof originalMethod !== "function") {
147+
throw new Error(`Not a function: "${_fullMethodName}"`);
148+
}
149+
150+
clonedService[method] = async function(this: typeof service) {
151+
const originalMethodArgs = arguments;
152+
const timeLeftUntilAllowed = rateLimiting.rateLimiterTick();
153+
const limitExceeded = timeLeftUntilAllowed > 0;
154+
155+
if (limitExceeded) {
156+
resolveServiceLogger.info([
157+
`delaying rate limited method calling: `,
158+
JSON.stringify({
159+
method: _fullMethodName,
160+
timeLeftUntilAllowed,
161+
rateLimitedMethodsCallCount,
162+
}),
163+
].join(""));
164+
165+
await asyncDelay(timeLeftUntilAllowed);
166+
}
167+
168+
resolveServiceLogger.debug(`queueing rate limited method: "${_fullMethodName}"`);
169+
170+
return rateLimitedApiCallingQueue.add(() => {
171+
resolveServiceLogger.verbose([
172+
`calling rate limited method: `,
173+
JSON.stringify({
174+
method: _fullMethodName,
175+
timeLeftUntilAllowed,
176+
rateLimitedMethodsCallCount,
177+
}),
178+
].join(""));
179+
180+
const result = originalMethod.apply(service, originalMethodArgs);
181+
rateLimitedMethodsCallCount++;
182+
return result;
183+
});
184+
} as any;
185+
}
89186

90-
return result;
187+
return clonedService;
91188
}

src/shared/model/electron.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import {InMemoryOptions, SyncOrAsyncLimiter} from "rolling-rate-limiter";
12
import {IpcRenderer} from "electron";
23

34
import {AccountType} from "src/shared/model/account";
@@ -7,7 +8,7 @@ export interface ElectronExposure {
78
ipcRendererTransport: Pick<IpcRenderer, "on" | "removeListener" | "send" | "sendToHost">;
89
webLogger: Logger;
910
require: {
10-
"rolling-rate-limiter": () => (...args: any[]) => (key: string) => number,
11+
"rolling-rate-limiter": () => (options: InMemoryOptions) => SyncOrAsyncLimiter;
1112
};
1213
}
1314

src/shared/model/options.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,13 @@ export interface Config extends BaseConfig, Partial<StoreModel.StoreEntity> {
2222
maximized?: boolean;
2323
bounds: { x?: number; y?: number; width: number; height: number; };
2424
};
25+
fetchingRateLimiting: {
26+
intervalMs: number;
27+
maxInInterval: number;
28+
};
29+
timeouts: {
30+
fetching: number;
31+
};
2532
}
2633

2734
export interface Settings extends Partial<StoreModel.StoreEntity> {

src/web/src/app/_accounts/accounts.effects.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,8 @@ export class AccountsEffects {
110110
return merge(
111111
merged$,
112112
this.api.webViewClient(webView, type, {finishPromise}).pipe(
113-
mergeMap((webViewClient) => merge(
113+
withLatestFrom(this.store.pipe(select(OptionsSelectors.CONFIG.timeouts))),
114+
mergeMap(([webViewClient, timeouts]) => merge(
114115
of(ACCOUNTS_ACTIONS.Patch({login, patch: {syncingActivated: true}})),
115116
this.store.pipe(
116117
select(OptionsSelectors.FEATURED.mainProcessNotification),
@@ -144,7 +145,7 @@ export class AccountsEffects {
144145
const client = type === "protonmail"
145146
? webViewClient as ReturnType<WebViewApi<typeof type>["buildClient"]>
146147
: webViewClient as ReturnType<WebViewApi<typeof type>["buildClient"]>;
147-
return client("buildDbPatch", {timeoutMs: ONE_SECOND_MS * 60 * 3})({
148+
return client("buildDbPatch", {timeoutMs: timeouts.fetching})({
148149
metadata: metadata as any, // TODO TS: get rid of "as any" casting
149150
zoneName,
150151
});

src/web/src/app/store/selectors/options.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,9 @@ export const FEATURED = {
1717

1818
export const CONFIG = {
1919
base: createSelector(FEATURED.config, pickBaseConfigProperties),
20-
compactLayout: createSelector(FEATURED.config, ({compactLayout}) => compactLayout),
21-
unreadNotifications: createSelector(FEATURED.config, ({unreadNotifications}) => unreadNotifications),
20+
compactLayout: createSelector(FEATURED.config, (c) => c.compactLayout),
21+
unreadNotifications: createSelector(FEATURED.config, (c) => c.unreadNotifications),
22+
timeouts: createSelector(FEATURED.config, (c) => c.timeouts),
2223
};
2324

2425
export const SETTINGS = (() => {

0 commit comments

Comments
 (0)