Page MenuHomePhabricator
Files F3363187

D9298.diff
No OneTemporary
Actions

D9298.diff
View Options

diff --git a/keyserver/src/socket/socket.js b/keyserver/src/socket/socket.js
--- a/keyserver/src/socket/socket.js
+++ b/keyserver/src/socket/socket.js
@@ -260,7 +260,11 @@
handleAsyncPromise(extendCookieLifespan(viewer.cookieID));
}
for (const response of serverResponses) {
- this.sendMessage(response);
+ // Normally it's an anti-pattern to await in sequence like this. But in
+ // this case, we have a requirement that this array of serverResponses
+ // is delivered in order. See here:
+ // https://github.com/CommE2E/comm/blob/101eb34481deb49c609bfd2c785f375886e52666/keyserver/src/socket/socket.js#L566-L568
+ await this.sendMessage(response);
}
if (clientSocketMessage.type === clientSocketMessageTypes.INITIAL) {
this.onSuccessfulConnection();
@@ -277,7 +281,7 @@
errorMessage.responseTo = responseTo;
}
this.markActivityOccurred();
- this.sendMessage(errorMessage);
+ await this.sendMessage(errorMessage);
return;
}
invariant(clientSocketMessage, 'should be set');
@@ -301,7 +305,7 @@
},
};
}
- this.sendMessage(authErrorMessage);
+ await this.sendMessage(authErrorMessage);
this.ws.close(4100, error.message);
return;
} else if (error.message === 'client_version_unsupported') {
@@ -338,19 +342,19 @@
},
};
}
- this.sendMessage(authErrorMessage);
+ await this.sendMessage(authErrorMessage);
this.ws.close(4101, error.message);
return;
}
if (error.payload) {
- this.sendMessage({
+ await this.sendMessage({
type: serverSocketMessageTypes.ERROR,
responseTo,
message: error.message,
payload: error.payload,
});
} else {
- this.sendMessage({
+ await this.sendMessage({
type: serverSocketMessageTypes.ERROR,
responseTo,
message: error.message,
@@ -379,7 +383,7 @@
}
};
- sendMessage = (message: ServerServerSocketMessage) => {
+ sendMessage = async (message: ServerServerSocketMessage) => {
invariant(
this.ws.readyState > 0,
"shouldn't send message until connection established",
@@ -405,7 +409,7 @@
return;
}
- const compressionResult = compressMessage(stringMessage);
+ const compressionResult = await compressMessage(stringMessage);
if (!compressionResult.compressed) {
this.ws.send(stringMessage);
return;
@@ -870,7 +874,7 @@
});
invariant(checkStateRequest, 'should be set');
- this.sendMessage({
+ await this.sendMessage({
type: serverSocketMessageTypes.REQUESTS,
payload: { serverRequests: [checkStateRequest] },
});
diff --git a/keyserver/src/utils/compress.js b/keyserver/src/utils/compress.js
--- a/keyserver/src/utils/compress.js
+++ b/keyserver/src/utils/compress.js
@@ -1,5 +1,6 @@
// @flow
+import { promisify } from 'util';
import zlib from 'zlib';
import type { CompressedData } from 'lib/types/compression-types.js';
@@ -11,15 +12,17 @@
};
const minimumSizeForCompression = 4096; // bytes
+const brotliCompress = promisify(zlib.brotliCompress);
+
type CompressionResult =
| { +compressed: true, +result: CompressedData }
| { +compressed: false, +result: string };
-function compressMessage(message: string): CompressionResult {
+async function compressMessage(message: string): Promise<CompressionResult> {
const bytesInMessage = new Blob([message]).size;
if (bytesInMessage < minimumSizeForCompression) {
return { compressed: false, result: message };
}
- const brotliResult = zlib.brotliCompressSync(message, brotliOptions);
+ const brotliResult = await brotliCompress(message, brotliOptions);
const base64Encoded = brotliResult.toString('base64');
const result = {
algo: 'brotli+base64',
diff --git a/lib/utils/sequential-promise-resolver.js b/lib/utils/sequential-promise-resolver.js
--- a/lib/utils/sequential-promise-resolver.js
+++ b/lib/utils/sequential-promise-resolver.js
@@ -1,11 +1,11 @@
// @flow
class SequentialPromiseResolver<Result> {
- onResolve: Result => mixed;
+ onResolve: Result => Promise<mixed>;
promises: Array<Promise<?Result>> = [];
currentlySpinning: boolean = false;
- constructor(onResolve: Result => mixed) {
+ constructor(onResolve: Result => Promise<mixed>) {
this.onResolve = onResolve;
}
@@ -22,9 +22,11 @@
let currentPromise = this.promises.shift();
while (currentPromise) {
+ // It's important that we await in sequence here as the messages must be
+ // delivered in order. For more context, see https://phab.comm.dev/D355
const result = await currentPromise;
if (result) {
- this.onResolve(result);
+ await this.onResolve(result);
}
currentPromise = this.promises.shift();
}

File Metadata

Mime Type
text/plain
Expires
Tue, Nov 26, 12:44 AM (21 h, 17 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
2581815
Default Alt Text
D9298.diff (4 KB)

Event Timeline