[0.12>0.13] fix token event not being canceled on promise rejection (#…

…1128)
microsoft · Feb 4, 2020 · 93c183f · 93c183f
2 parents 4897a55 + 9b5adb6
commit 93c183f
Showing 1 changed file with 8 additions and 25 deletions.
diff --git a/...socket-storage/src/odspDocumentService.ts → ...socket-storage/src/OdspDocumentService.ts b/...socket-storage/src/odspDocumentService.ts → ...socket-storage/src/OdspDocumentService.ts
@@ -96,11 +96,11 @@ export class OdspDocumentService implements IDocumentService {
                 // (fluid-fetcher)
                 this.logger.sendTelemetryEvent({ eventName: "StorageTokenRefresh" });
             }
-            const event = PerformanceEvent.start(this.logger,
+            const event = PerformanceEvent.start(this.logger, 
                 { eventName: `${name || "OdspDocumentService"}_GetToken` });
-            let token: Promise<string | null>;
+            let token: string | null;
             try {
-                token = getStorageToken(this.siteUrl, refresh);
+                token = await getStorageToken(this.siteUrl, refresh);
             } catch (error) {
                 event.cancel({}, error);
                 throw error;
@@ -165,22 +165,7 @@ export class OdspDocumentService implements IDocumentService {
     public async connectToDeltaStream(client: IClient, mode: ConnectionMode): Promise<IDocumentDeltaConnection> {
         // Attempt to connect twice, in case we used expired token.
         return getWithRetryForTokenRefresh<IDocumentDeltaConnection>(async (refresh: boolean) => {
-            const [websocketEndpoint, webSocketToken, io] =
-                await Promise.all([this.joinSession(), this.getWebsocketToken(refresh), this.socketIOClientP]);
-
-            // This check exists because of a typescript bug.
-            // Issue: https://github.com/microsoft/TypeScript/issues/33752
-            // The TS team has plans to fix this in the 3.8 release
-            if (!websocketEndpoint) {
-                throw new Error("websocket endpoint should be defined");
-            }
-
-            // This check exists because of a typescript bug.
-            // Issue: https://github.com/microsoft/TypeScript/issues/33752
-            // The TS team has plans to fix this in the 3.8 release
-            if (!io) {
-                throw new Error("websocket endpoint should be defined");
-            }
+            const [websocketEndpoint, webSocketToken, io] = await Promise.all([this.joinSession(), this.getWebsocketToken(refresh), this.socketIOClientP]);
 
             return this.connectToDeltaStreamWithRetry(
                 websocketEndpoint.tenantId,
@@ -251,7 +236,7 @@ export class OdspDocumentService implements IDocumentService {
     }
 
     /**
-     * Test if we deal with NetworkError object and if it has enough information to make a call
+     * Test if we deal with INetworkError / NetworkError object and if it has enough information to make a call
      * If in doubt, allow retries
      *
      * @param error - error object
@@ -273,6 +258,7 @@ export class OdspDocumentService implements IDocumentService {
      * @param url - websocket URL
      * @param url2 - alternate websocket URL
      */
+    // tslint:disable-next-line: max-func-body-length
     private async connectToDeltaStreamWithRetry(
         tenantId: string,
         websocketId: string,
@@ -317,6 +303,7 @@ export class OdspDocumentService implements IDocumentService {
                 io,
                 client,
                 mode,
+                // tslint:disable-next-line: no-non-null-assertion
                 url2!,
                 20000,
                 this.logger,
@@ -327,13 +314,11 @@ export class OdspDocumentService implements IDocumentService {
                 });
 
                 return connection;
-                // eslint-disable-next-line @typescript-eslint/promise-function-async
             }).catch((connectionError) => {
                 const endAfd = performanceNow();
                 localStorage.removeItem(lastAfdConnectionTimeMsKey);
                 // Retry on non-AFD URL
                 if (this.canRetryOnError(connectionError)) {
-                    // eslint-disable-next-line max-len
                     debug(`Socket connection error on AFD URL (cached). Error was [${connectionError}]. Retry on non-AFD URL: ${url}`);
 
                     return OdspDocumentDeltaConnection.create(
@@ -383,11 +368,9 @@ export class OdspDocumentService implements IDocumentService {
         ).then((connection) => {
             logger.sendTelemetryEvent({ eventName: "UsedNonAfdUrl" });
             return connection;
-            // eslint-disable-next-line @typescript-eslint/promise-function-async
         }).catch((connectionError) => {
             const endNonAfd = performanceNow();
             if (hasUrl2 && this.canRetryOnError(connectionError)) {
-                // eslint-disable-next-line max-len
                 debug(`Socket connection error on non-AFD URL. Error was [${connectionError}]. Retry on AFD URL: ${url2}`);
 
                 return OdspDocumentDeltaConnection.create(
@@ -397,14 +380,14 @@ export class OdspDocumentService implements IDocumentService {
                     io,
                     client,
                     mode,
+                    // tslint:disable-next-line: no-non-null-assertion
                     url2!,
                     20000,
                     this.logger,
                 ).then((connection) => {
                     // Refresh AFD cache
                     const cacheResult = this.writeLocalStorage(lastAfdConnectionTimeMsKey, Date.now().toString());
                     if (cacheResult) {
-                        // eslint-disable-next-line max-len
                         debug(`Cached AFD connection time. Expiring in ${new Date(Number(localStorage.getItem(lastAfdConnectionTimeMsKey)) + afdUrlConnectExpirationMs)}`);
                     }
                     logger.sendPerformanceEvent({