Skip to content

Commit 6c43ec6

Browse files
[Fix] Unhandled exception on reconnect (#106)
1 parent 5b4b560 commit 6c43ec6

File tree

18 files changed

+428
-149
lines changed

18 files changed

+428
-149
lines changed

.changeset/clean-moles-vanish.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@journeyapps/powersync-sdk-web': patch
3+
---
4+
5+
Fixed shared sync broadcast logger sanitization and error handling. Added ability to disabled broadcast logging in `WebPowerSyncFlags`.

.changeset/warm-bottles-hammer.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@journeyapps/powersync-sdk-common': patch
3+
---
4+
5+
Fixed potential unhandled exception when aborting stream fetch request for `/sync/stream` endpoint

demos/react-native-supabase-todolist/ios/Podfile.lock

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1148,18 +1148,18 @@ DEPENDENCIES:
11481148
- boost (from `../node_modules/react-native/third-party-podspecs/boost.podspec`)
11491149
- DoubleConversion (from `../node_modules/react-native/third-party-podspecs/DoubleConversion.podspec`)
11501150
- EASClient (from `../../../node_modules/expo-eas-client/ios`)
1151-
- EXConstants (from `../../../node_modules/expo-constants/ios`)
1151+
- EXConstants (from `../node_modules/expo-constants/ios`)
11521152
- EXFont (from `../../../node_modules/expo-font/ios`)
11531153
- EXJSONUtils (from `../../../node_modules/expo-json-utils/ios`)
11541154
- EXManifests (from `../../../node_modules/expo-manifests/ios`)
1155-
- Expo (from `../../../node_modules/expo`)
1155+
- Expo (from `../node_modules/expo`)
11561156
- ExpoCamera (from `../../../node_modules/expo-camera/ios`)
1157-
- ExpoFileSystem (from `../../../node_modules/expo-file-system/ios`)
1157+
- ExpoFileSystem (from `../node_modules/expo-file-system/ios`)
11581158
- ExpoHead (from `../node_modules/expo-router/ios`)
11591159
- ExpoKeepAwake (from `../../../node_modules/expo-keep-awake/ios`)
11601160
- ExpoModulesCore (from `../../../node_modules/expo-modules-core`)
11611161
- ExpoSecureStore (from `../../../node_modules/expo-secure-store/ios`)
1162-
- EXSplashScreen (from `../../../node_modules/expo-splash-screen/ios`)
1162+
- EXSplashScreen (from `../node_modules/expo-splash-screen/ios`)
11631163
- EXStructuredHeaders (from `../../../node_modules/expo-structured-headers/ios`)
11641164
- EXUpdates (from `../../../node_modules/expo-updates/ios`)
11651165
- EXUpdatesInterface (from `../../../node_modules/expo-updates-interface/ios`)
@@ -1192,9 +1192,9 @@ DEPENDENCIES:
11921192
- React-logger (from `../node_modules/react-native/ReactCommon/logger`)
11931193
- React-Mapbuffer (from `../node_modules/react-native/ReactCommon`)
11941194
- react-native-encrypted-storage (from `../../../node_modules/react-native-encrypted-storage`)
1195-
- react-native-get-random-values (from `../../../node_modules/react-native-get-random-values`)
1196-
- "react-native-quick-sqlite (from `../../../node_modules/@journeyapps/react-native-quick-sqlite`)"
1197-
- react-native-safe-area-context (from `../../../node_modules/react-native-safe-area-context`)
1195+
- react-native-get-random-values (from `../node_modules/react-native-get-random-values`)
1196+
- "react-native-quick-sqlite (from `../node_modules/@journeyapps/react-native-quick-sqlite`)"
1197+
- react-native-safe-area-context (from `../node_modules/react-native-safe-area-context`)
11981198
- React-nativeconfig (from `../node_modules/react-native/ReactCommon`)
11991199
- React-NativeModulesApple (from `../node_modules/react-native/ReactCommon/react/nativemodule/core/platform/ios`)
12001200
- React-perflogger (from `../node_modules/react-native/ReactCommon/reactperflogger`)
@@ -1216,9 +1216,9 @@ DEPENDENCIES:
12161216
- React-utils (from `../node_modules/react-native/ReactCommon/react/utils`)
12171217
- ReactCommon/turbomodule/core (from `../node_modules/react-native/ReactCommon`)
12181218
- "RNCMaskedView (from `../../../node_modules/@react-native-community/masked-view`)"
1219-
- RNGestureHandler (from `../../../node_modules/react-native-gesture-handler`)
1220-
- RNReanimated (from `../../../node_modules/react-native-reanimated`)
1221-
- RNScreens (from `../../../node_modules/react-native-screens`)
1219+
- RNGestureHandler (from `../node_modules/react-native-gesture-handler`)
1220+
- RNReanimated (from `../node_modules/react-native-reanimated`)
1221+
- RNScreens (from `../node_modules/react-native-screens`)
12221222
- RNVectorIcons (from `../../../node_modules/react-native-vector-icons`)
12231223
- Yoga (from `../node_modules/react-native/ReactCommon/yoga`)
12241224

@@ -1240,19 +1240,19 @@ EXTERNAL SOURCES:
12401240
EASClient:
12411241
:path: "../../../node_modules/expo-eas-client/ios"
12421242
EXConstants:
1243-
:path: "../../../node_modules/expo-constants/ios"
1243+
:path: "../node_modules/expo-constants/ios"
12441244
EXFont:
12451245
:path: "../../../node_modules/expo-font/ios"
12461246
EXJSONUtils:
12471247
:path: "../../../node_modules/expo-json-utils/ios"
12481248
EXManifests:
12491249
:path: "../../../node_modules/expo-manifests/ios"
12501250
Expo:
1251-
:path: "../../../node_modules/expo"
1251+
:path: "../node_modules/expo"
12521252
ExpoCamera:
12531253
:path: "../../../node_modules/expo-camera/ios"
12541254
ExpoFileSystem:
1255-
:path: "../../../node_modules/expo-file-system/ios"
1255+
:path: "../node_modules/expo-file-system/ios"
12561256
ExpoHead:
12571257
:path: "../node_modules/expo-router/ios"
12581258
ExpoKeepAwake:
@@ -1262,7 +1262,7 @@ EXTERNAL SOURCES:
12621262
ExpoSecureStore:
12631263
:path: "../../../node_modules/expo-secure-store/ios"
12641264
EXSplashScreen:
1265-
:path: "../../../node_modules/expo-splash-screen/ios"
1265+
:path: "../node_modules/expo-splash-screen/ios"
12661266
EXStructuredHeaders:
12671267
:path: "../../../node_modules/expo-structured-headers/ios"
12681268
EXUpdates:
@@ -1323,11 +1323,11 @@ EXTERNAL SOURCES:
13231323
react-native-encrypted-storage:
13241324
:path: "../../../node_modules/react-native-encrypted-storage"
13251325
react-native-get-random-values:
1326-
:path: "../../../node_modules/react-native-get-random-values"
1326+
:path: "../node_modules/react-native-get-random-values"
13271327
react-native-quick-sqlite:
1328-
:path: "../../../node_modules/@journeyapps/react-native-quick-sqlite"
1328+
:path: "../node_modules/@journeyapps/react-native-quick-sqlite"
13291329
react-native-safe-area-context:
1330-
:path: "../../../node_modules/react-native-safe-area-context"
1330+
:path: "../node_modules/react-native-safe-area-context"
13311331
React-nativeconfig:
13321332
:path: "../node_modules/react-native/ReactCommon"
13331333
React-NativeModulesApple:
@@ -1371,11 +1371,11 @@ EXTERNAL SOURCES:
13711371
RNCMaskedView:
13721372
:path: "../../../node_modules/@react-native-community/masked-view"
13731373
RNGestureHandler:
1374-
:path: "../../../node_modules/react-native-gesture-handler"
1374+
:path: "../node_modules/react-native-gesture-handler"
13751375
RNReanimated:
1376-
:path: "../../../node_modules/react-native-reanimated"
1376+
:path: "../node_modules/react-native-reanimated"
13771377
RNScreens:
1378-
:path: "../../../node_modules/react-native-screens"
1378+
:path: "../node_modules/react-native-screens"
13791379
RNVectorIcons:
13801380
:path: "../../../node_modules/react-native-vector-icons"
13811381
Yoga:
@@ -1465,4 +1465,4 @@ SPEC CHECKSUMS:
14651465

14661466
PODFILE CHECKSUM: 91f1b09fe73837e9fdaecdd06e4916926352d556
14671467

1468-
COCOAPODS: 1.15.2
1468+
COCOAPODS: 1.13.0

packages/powersync-sdk-common/src/client/AbstractPowerSyncDatabase.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ export abstract class AbstractPowerSyncDatabase extends BaseObserver<PowerSyncDB
268268

269269
await this.syncStreamImplementation.waitForReady();
270270
this.syncStreamImplementation.triggerCrudUpload();
271-
this.syncStreamImplementation.connect();
271+
await this.syncStreamImplementation.connect();
272272
}
273273

274274
/**
@@ -277,6 +277,7 @@ export abstract class AbstractPowerSyncDatabase extends BaseObserver<PowerSyncDB
277277
* Use {@link connect} to connect again.
278278
*/
279279
async disconnect() {
280+
await this.waitForReady();
280281
await this.syncStreamImplementation?.disconnect();
281282
this.syncStatusListenerDisposer?.();
282283
await this.syncStreamImplementation?.dispose();

packages/powersync-sdk-common/src/client/sync/stream/AbstractStreamingSyncImplementation.ts

Lines changed: 104 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,21 @@ import Logger, { ILogger } from 'js-logger';
44

55
import {
66
BucketRequest,
7+
StreamingSyncLine,
8+
StreamingSyncRequest,
79
isStreamingKeepalive,
810
isStreamingSyncCheckpoint,
911
isStreamingSyncCheckpointComplete,
1012
isStreamingSyncCheckpointDiff,
11-
isStreamingSyncData,
12-
StreamingSyncLine,
13-
StreamingSyncRequest
13+
isStreamingSyncData
1414
} from './streaming-sync-types';
1515
import { AbstractRemote } from './AbstractRemote';
1616
import ndjsonStream from 'can-ndjson-stream';
1717
import { BucketChecksum, BucketStorageAdapter, Checkpoint } from '../bucket/BucketStorageAdapter';
1818
import { SyncStatus, SyncStatusOptions } from '../../../db/crud/SyncStatus';
1919
import { SyncDataBucket } from '../bucket/SyncDataBucket';
2020
import { BaseObserver, BaseListener, Disposable } from '../../../utils/BaseObserver';
21+
import { AbortOperation } from '../../../utils/AbortOperation';
2122

2223
export enum LockType {
2324
CRUD = 'crud',
@@ -47,6 +48,14 @@ export interface AbstractStreamingSyncImplementationOptions {
4748
}
4849

4950
export interface StreamingSyncImplementationListener extends BaseListener {
51+
/**
52+
* Triggered whenever a status update has been attempted to be made or
53+
* refreshed.
54+
*/
55+
statusUpdated?: ((statusUpdate: SyncStatusOptions) => void) | undefined;
56+
/**
57+
* Triggers whenever the status' members have changed in value
58+
*/
5059
statusChanged?: ((status: SyncStatus) => void) | undefined;
5160
}
5261

@@ -86,6 +95,7 @@ export abstract class AbstractStreamingSyncImplementation
8695
protected options: AbstractStreamingSyncImplementationOptions;
8796
protected abortController: AbortController | null;
8897
protected crudUpdateListener?: () => void;
98+
protected streamingSyncPromise?: Promise<void>;
8999

90100
syncStatus: SyncStatus;
91101
triggerCrudUpload: () => void;
@@ -224,16 +234,53 @@ export abstract class AbstractStreamingSyncImplementation
224234
if (this.abortController) {
225235
await this.disconnect();
226236
}
237+
227238
this.abortController = new AbortController();
228-
this.streamingSync(this.abortController.signal);
229-
return this.waitForStatus({ connected: true });
239+
this.streamingSyncPromise = this.streamingSync(this.abortController.signal);
240+
241+
// Return a promise that resolves when the connection status is updated
242+
return new Promise<void>((resolve) => {
243+
const l = this.registerListener({
244+
statusUpdated: (update) => {
245+
// This is triggered as soon as a connection is read from
246+
if (typeof update.connected == 'undefined') {
247+
// only concern with connection updates
248+
return;
249+
}
250+
251+
if (update.connected == false) {
252+
/**
253+
* This function does not reject if initial connect attempt failed
254+
*/
255+
this.logger.warn('Initial connect attempt did not successfully connect to server');
256+
}
257+
258+
resolve();
259+
l();
260+
}
261+
});
262+
});
230263
}
231264

232265
async disconnect(): Promise<void> {
233266
if (!this.abortController) {
234-
throw new Error('Disconnect not possible');
267+
return;
268+
}
269+
270+
// This might be called multiple times
271+
if (!this.abortController.signal.aborted) {
272+
this.abortController.abort(new AbortOperation('Disconnect has been requested'));
235273
}
236-
this.abortController.abort('Disconnected');
274+
275+
// Await any pending operations before completing the disconnect operation
276+
try {
277+
await this.streamingSyncPromise;
278+
} catch (ex) {
279+
// The operation might have failed, all we care about is if it has completed
280+
this.logger.warn(ex);
281+
}
282+
this.streamingSyncPromise = undefined;
283+
237284
this.abortController = null;
238285
this.updateSyncStatus({ connected: false });
239286
}
@@ -261,7 +308,11 @@ export abstract class AbstractStreamingSyncImplementation
261308
let nestedAbortController = new AbortController();
262309

263310
signal.addEventListener('abort', () => {
264-
nestedAbortController.abort();
311+
/**
312+
* A request for disconnect was received upstream. Relay the request
313+
* to the nested abort controller.
314+
*/
315+
nestedAbortController.abort(signal?.reason ?? new AbortOperation('Received command to disconnect from upstream'));
265316
this.crudUpdateListener?.();
266317
this.crudUpdateListener = undefined;
267318
this.updateSyncStatus({
@@ -272,34 +323,58 @@ export abstract class AbstractStreamingSyncImplementation
272323
});
273324
});
274325

326+
/**
327+
* This loops runs until [retry] is false or the abort signal is set to aborted.
328+
* Aborting the nestedAbortController will:
329+
* - Abort any pending fetch requests
330+
* - Close any sync stream ReadableStreams (which will also close any established network requests)
331+
*/
275332
while (true) {
276333
try {
277334
if (signal?.aborted) {
278335
break;
279336
}
280337
const { retry } = await this.streamingSyncIteration(nestedAbortController.signal);
281338
if (!retry) {
339+
/**
340+
* A sync error ocurred that we cannot recover from here.
341+
* This loop must terminate.
342+
* The nestedAbortController will close any open network requests and streams below.
343+
*/
282344
break;
283345
}
284346
// Continue immediately
285347
} catch (ex) {
286-
this.logger.error(ex);
348+
/**
349+
* Either:
350+
* - A network request failed with a failed connection or not OKAY response code.
351+
* - There was a sync processing error.
352+
* This loop will retry.
353+
* The nested abort controller will cleanup any open network requests and streams.
354+
* The WebRemote should only abort pending fetch requests or close active Readable streams.
355+
*/
356+
if (ex instanceof AbortOperation) {
357+
this.logger.warn(ex);
358+
} else {
359+
this.logger.error(ex);
360+
}
361+
await this.delayRetry();
362+
} finally {
363+
if (!signal.aborted) {
364+
nestedAbortController.abort(new AbortOperation('Closing sync stream network requests before retry.'));
365+
nestedAbortController = new AbortController();
366+
}
367+
287368
this.updateSyncStatus({
288369
connected: false
289370
});
371+
290372
// On error, wait a little before retrying
291-
await this.delayRetry();
292-
} finally {
293-
// Abort any open network requests. Create a new nested controller for retry.
294-
nestedAbortController.abort();
295-
nestedAbortController = new AbortController();
296373
}
297374
}
298375

299376
// Mark as disconnected if here
300-
if (this.abortController) {
301-
await this.disconnect();
302-
}
377+
this.updateSyncStatus({ connected: false });
303378
}
304379

305380
protected async streamingSyncIteration(signal: AbortSignal, progress?: () => void): Promise<{ retry?: boolean }> {
@@ -336,15 +411,6 @@ export abstract class AbstractStreamingSyncImplementation
336411
},
337412
signal
338413
)) {
339-
// A connection is active and messages are being received
340-
if (!this.syncStatus.connected) {
341-
// There is a connection now
342-
Promise.resolve().then(() => this.triggerCrudUpload());
343-
this.updateSyncStatus({
344-
connected: true
345-
});
346-
}
347-
348414
if (isStreamingSyncCheckpoint(line)) {
349415
targetCheckpoint = line.checkpoint;
350416
const bucketsToDelete = new Set<string>(bucketSet);
@@ -476,6 +542,14 @@ export abstract class AbstractStreamingSyncImplementation
476542
signal?: AbortSignal
477543
): AsyncGenerator<StreamingSyncLine> {
478544
const body = await this.options.remote.postStreaming('/sync/stream', req, {}, signal);
545+
546+
// A connection is active
547+
// There is a connection now
548+
Promise.resolve().then(() => this.triggerCrudUpload());
549+
this.updateSyncStatus({
550+
connected: true
551+
});
552+
479553
const stream = ndjsonStream(body);
480554
const reader = stream.getReader();
481555

@@ -505,8 +579,12 @@ export abstract class AbstractStreamingSyncImplementation
505579

506580
if (!this.syncStatus.isEqual(updatedStatus)) {
507581
this.syncStatus = updatedStatus;
582+
// Only trigger this is there was a change
508583
this.iterateListeners((cb) => cb.statusChanged?.(updatedStatus));
509584
}
585+
586+
// trigger this for all updates
587+
this.iterateListeners((cb) => cb.statusUpdated?.(options));
510588
}
511589

512590
private async delayRetry() {

packages/powersync-sdk-common/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,6 @@ export * from './db/DBAdapter';
2727
export * from './db/Column';
2828
export * from './db/schema/TableV2';
2929

30+
export * from './utils/AbortOperation';
3031
export * from './utils/BaseObserver';
3132
export * from './utils/strings';

0 commit comments

Comments
 (0)