Skip to content

Commit 76ebc7c

Browse files
committed
Fix sending duplicate revisions
1 parent 802de38 commit 76ebc7c

File tree

4 files changed

+38
-6
lines changed

4 files changed

+38
-6
lines changed
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { getTextOperation } from '@codesandbox/common/lib/utils/diff';
2+
import { CodeSandboxOTClient } from './clients';
3+
4+
describe('OTClient', () => {
5+
it("it doesn't acknowledge the same revision twice", async () => {
6+
const client = new CodeSandboxOTClient(
7+
0,
8+
'test',
9+
(revision, operation) => Promise.resolve(),
10+
operation => {}
11+
);
12+
13+
const op = getTextOperation('ab', 'a');
14+
15+
// The case we're trying to solve here:
16+
// (1) applyClient -> (no ack received)
17+
// -----
18+
// serverReconnect
19+
// (2) applyClient. Sent by serverReconnect
20+
// (1) resolves. Resolved because of Phoenix
21+
// (2) resolves. Resolved because of server reconnect
22+
23+
await client.sendOperation(0, op);
24+
await client.sendOperation(0, op);
25+
});
26+
});

packages/app/src/app/overmind/effects/live/clients.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export type SendOperation = (
1111

1212
export type ApplyOperation = (moduleShortid: string, operation: any) => void;
1313

14-
class CodeSandboxOTClient extends OTClient {
14+
export class CodeSandboxOTClient extends OTClient {
1515
/*
1616
We need to be able to wait for a client to go intro synchronized
1717
state. The reason is that we want to send a "save" event when the
@@ -34,6 +34,7 @@ class CodeSandboxOTClient extends OTClient {
3434
this.onApplyOperation = onApplyOperation;
3535
}
3636

37+
lastAcknowledgedRevision = null;
3738
sendOperation(revision, operation) {
3839
// Whenever we send an operation we enable the blocker
3940
// that lets us wait for its resolvment when moving back
@@ -42,7 +43,7 @@ class CodeSandboxOTClient extends OTClient {
4243
this.awaitSynchronized = blocker();
4344
}
4445

45-
this.onSendOperation(revision, operation)
46+
return this.onSendOperation(revision, operation)
4647
.then(() => {
4748
logBreadcrumb({
4849
type: 'ot',
@@ -52,8 +53,10 @@ class CodeSandboxOTClient extends OTClient {
5253
operation,
5354
})}`,
5455
});
55-
// We only acknowledge valig revisions
56-
if (this.revision === revision) {
56+
57+
// We make sure to not acknowledge the same revision twice
58+
if (this.lastAcknowledgedRevision < revision) {
59+
this.lastAcknowledgedRevision = revision;
5760
this.serverAck();
5861
}
5962
})
@@ -64,6 +67,8 @@ class CodeSandboxOTClient extends OTClient {
6467
if (this.awaitSynchronized) {
6568
this.awaitSynchronized.reject(error);
6669
}
70+
71+
throw error;
6772
});
6873
}
6974

packages/common/src/utils/analytics/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export async function identify(key: string, value: any) {
4040
}
4141

4242
export async function setAnonymousId() {
43-
if (!DO_NOT_TRACK_ENABLED) {
43+
if (!DO_NOT_TRACK_ENABLED && typeof localStorage !== 'undefined') {
4444
let anonymousUid = localStorage.getItem(ANONYMOUS_UID_KEY);
4545

4646
if (!anonymousUid) {

packages/common/src/utils/analytics/utils.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ export const DO_NOT_TRACK_ENABLED =
7676
global.navigator.doNotTrack === '1' ||
7777
// @ts-ignore
7878
global.navigator.msDoNotTrack === '1' ||
79-
localStorage.getItem('DO_NOT_TRACK_ENABLED') ||
79+
(typeof localStorage !== 'undefined' &&
80+
localStorage.getItem('DO_NOT_TRACK_ENABLED')) ||
8081
process.env.NODE_ENV === 'development' ||
8182
process.env.STAGING
8283
);

0 commit comments

Comments
 (0)