Skip to content

Commit

Permalink
Wait on correct Promise in subscription
Browse files Browse the repository at this point in the history
I'm not a Promise expert, but the previous code at least read like it was
possible for the Promise returned by subscribe to resolve once all the
this.pubsub.subscribe Promises resolved but before all of the "push onto
this.subscriptions" Promises resolved.  If unsubscribe was called too quickly,
you could in theory end up calling one of those "push" functions after the
`delete this.subscriptions[subId]` line and crashing.

In practice the old code does appear to work but this code more accurately shows
the intentions.
  • Loading branch information
glasser committed Dec 22, 2016
1 parent b3a6c8d commit af3dfef
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions src/pubsub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,10 @@ export class SubscriptionManager {
}

// 3. subscribe and keep the subscription id
const subsPromise = this.pubsub.subscribe(triggerName, onMessage, channelOptions);
subsPromise.then(id => this.subscriptions[externalSubscriptionId].push(id));

subscriptionPromises.push(subsPromise);
subscriptionPromises.push(
this.pubsub.subscribe(triggerName, onMessage, channelOptions)
.then(id => this.subscriptions[externalSubscriptionId].push(id))
);
});

// Resolve the promise with external sub id only after all subscriptions completed
Expand Down

0 comments on commit af3dfef

Please sign in to comment.