Skip to content

Commit

Permalink
fix: Network monitoring only active when the connection is intended t…
Browse files Browse the repository at this point in the history
…o be open
  • Loading branch information
stocaaro committed Jul 29, 2022
1 parent a1022c3 commit 386564d
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 26 deletions.
21 changes: 5 additions & 16 deletions packages/pubsub/__tests__/ConnectionStateMonitor.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,11 @@ describe('ConnectionStateMonitor', () => {
beforeEach(() => {
const spyon = jest
.spyOn(Reachability.prototype, 'networkMonitor')
.mockImplementationOnce(
() =>
new Observable(observer => {
reachabilityObserver = observer;
})
);
.mockImplementationOnce(() => {
return new Observable(observer => {
reachabilityObserver = observer;
});
});
});

describe('when the network is connected', () => {
Expand All @@ -57,11 +56,6 @@ describe('ConnectionStateMonitor', () => {
subscription = monitor.connectionStateObservable.subscribe(value => {
observedStates.push(value);
});
monitor.enableNetworkMonitoring();
});

afterEach(() => {
monitor.disableNetworkMonitoring();
});

test('connection states starts out disconnected', () => {
Expand Down Expand Up @@ -223,11 +217,6 @@ describe('ConnectionStateMonitor', () => {
subscription = monitor.connectionStateObservable.subscribe(value => {
observedStates.push(value);
});
monitor.enableNetworkMonitoring();
});

afterEach(() => {
monitor.disableNetworkMonitoring();
});

test('starts out disconnected', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,6 @@ export class AWSAppSyncRealTimeProvider extends AbstractPubSubProvider {
): Observable<any> {
const appSyncGraphqlEndpoint = options?.appSyncGraphqlEndpoint;

this.connectionStateMonitor.enableNetworkMonitoring();

return new Observable(observer => {
if (!options || !appSyncGraphqlEndpoint) {
observer.error({
Expand Down Expand Up @@ -175,7 +173,6 @@ export class AWSAppSyncRealTimeProvider extends AbstractPubSubProvider {
],
});
this.connectionStateMonitor.record(CONNECTION_CHANGE.CLOSED);
this.connectionStateMonitor.disableNetworkMonitoring();
observer.complete();
});

Expand Down Expand Up @@ -293,7 +290,6 @@ export class AWSAppSyncRealTimeProvider extends AbstractPubSubProvider {
logger.debug({ err });
const message = err['message'] ?? '';
this.connectionStateMonitor.record(CONNECTION_CHANGE.CLOSED);
this.connectionStateMonitor.disableNetworkMonitoring();
observer.error({
errors: [
{
Expand Down Expand Up @@ -420,7 +416,6 @@ export class AWSAppSyncRealTimeProvider extends AbstractPubSubProvider {
this.awsRealTimeSocket = undefined;
this.socketStatus = SOCKET_STATUS.CLOSED;
this.connectionStateMonitor.record(CONNECTION_CHANGE.CLOSED);
this.connectionStateMonitor.disableNetworkMonitoring();
}
}

Expand Down Expand Up @@ -540,7 +535,6 @@ export class AWSAppSyncRealTimeProvider extends AbstractPubSubProvider {
this.subscriptionObserverMap.clear();
if (this.awsRealTimeSocket) {
this.connectionStateMonitor.record(CONNECTION_CHANGE.CLOSED);
this.connectionStateMonitor.disableNetworkMonitoring();
this.awsRealTimeSocket.close();
}

Expand Down
15 changes: 11 additions & 4 deletions packages/pubsub/src/utils/ConnectionStateMonitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ export class ConnectionStateMonitor {
}

/**
* Turn network state monitoring on
* Turn network state monitoring on if it isn't on already
*/
public enableNetworkMonitoring() {
private enableNetworkMonitoring() {
// Maintain the network state based on the reachability monitor
if (this._networkMonitoringSubscription === undefined) {
this._networkMonitoringSubscription = new Reachability()
Expand All @@ -96,9 +96,9 @@ export class ConnectionStateMonitor {
}

/**
* Turn network state monitoring off
* Turn network state monitoring off if it isn't off already
*/
public disableNetworkMonitoring() {
private disableNetworkMonitoring() {
this._networkMonitoringSubscription?.unsubscribe();
this._networkMonitoringSubscription = undefined;
}
Expand Down Expand Up @@ -131,6 +131,13 @@ export class ConnectionStateMonitor {
* Updates local connection state and emits the full state to the observer.
*/
record(statusUpdates: Partial<LinkedConnectionStates>) {
// Maintain the network monitor
if (statusUpdates.intendedConnectionState === 'connected') {
this.enableNetworkMonitoring();
} else if (statusUpdates.intendedConnectionState === 'disconnected') {
this.disableNetworkMonitoring();
}

// Maintain the socket state
const newSocketStatus = {
...this._linkedConnectionState,
Expand Down

0 comments on commit 386564d

Please sign in to comment.