Port two fixes to the release branch

CHANGELOG.md

@@ -1,5 +1,68 @@
 # Changelog
 
+## 2020.4.1 (27 April 2020)
+
+### Fixes
+
+1. Fix issue where downloading ipywidgets from the CDN might be busy.
+   ([#11274](https://github.com/Microsoft/vscode-python/issues/11274))
+1. Error: Timeout is shown after running any widget more than once.
+   ([#11334](https://github.com/Microsoft/vscode-python/issues/11334))
+
+### Thanks
+
+Thanks to the following projects which we fully rely on to provide some of
+our features:
+
+-   [debugpy](https://pypi.org/project/debugpy/)
+-   [isort](https://pypi.org/project/isort/)
+-   [jedi](https://pypi.org/project/jedi/)
+    and [parso](https://pypi.org/project/parso/)
+-   [Microsoft Python Language Server](https://github.com/microsoft/python-language-server)
+-   [ptvsd](https://pypi.org/project/ptvsd/)
+-   [exuberant ctags](http://ctags.sourceforge.net/) (user-installed)
+-   [rope](https://pypi.org/project/rope/) (user-installed)
+
+Also thanks to the various projects we provide integrations with which help
+make this extension useful:
+
+-   Debugging support:
+    [Django](https://pypi.org/project/Django/),
+    [Flask](https://pypi.org/project/Flask/),
+    [gevent](https://pypi.org/project/gevent/),
+    [Jinja](https://pypi.org/project/Jinja/),
+    [Pyramid](https://pypi.org/project/pyramid/),
+    [PySpark](https://pypi.org/project/pyspark/),
+    [Scrapy](https://pypi.org/project/Scrapy/),
+    [Watson](https://pypi.org/project/Watson/)
+-   Formatting:
+    [autopep8](https://pypi.org/project/autopep8/),
+    [black](https://pypi.org/project/black/),
+    [yapf](https://pypi.org/project/yapf/)
+-   Interpreter support:
+    [conda](https://conda.io/),
+    [direnv](https://direnv.net/),
+    [pipenv](https://pypi.org/project/pipenv/),
+    [pyenv](https://github.com/pyenv/pyenv),
+    [venv](https://docs.python.org/3/library/venv.html#module-venv),
+    [virtualenv](https://pypi.org/project/virtualenv/)
+-   Linting:
+    [bandit](https://pypi.org/project/bandit/),
+    [flake8](https://pypi.org/project/flake8/),
+    [mypy](https://pypi.org/project/mypy/),
+    [prospector](https://pypi.org/project/prospector/),
+    [pylint](https://pypi.org/project/pylint/),
+    [pydocstyle](https://pypi.org/project/pydocstyle/),
+    [pylama](https://pypi.org/project/pylama/)
+-   Testing:
+    [nose](https://pypi.org/project/nose/),
+    [pytest](https://pypi.org/project/pytest/),
+    [unittest](https://docs.python.org/3/library/unittest.html#module-unittest)
+
+And finally thanks to the [Python](https://www.python.org/) development team and
+community for creating a fantastic programming language and community to be a
+part of!
+
 ## 2020.4.0 (20 April 2020)
 
 ### Enhancements

src/client/datascience/constants.ts

@@ -404,6 +404,7 @@ export namespace Identifiers {
     export const InteractiveWindowIdentity = 'history://EC155B3B-DC18-49DC-9E99-9A948AA2F27B';
     export const InteractiveWindowIdentityScheme = 'history';
     export const DefaultCodeCellMarker = '# %%';
+    export const DefaultCommTarget = 'jupyter.widget';
 }
 
 export namespace CodeSnippits {

src/client/datascience/ipywidgets/cdnWidgetScriptSourceProvider.ts

@@ -3,16 +3,25 @@
 
 'use strict';
 
-import { traceWarning } from '../../common/logger';
+import * as fs from 'fs-extra';
+import { sha256 } from 'hash.js';
+import * as path from 'path';
+import request from 'request';
+import { Uri } from 'vscode';
+import { traceError, traceInfo } from '../../common/logger';
+import { IFileSystem, TemporaryFile } from '../../common/platform/types';
 import { IConfigurationService, IHttpClient, WidgetCDNs } from '../../common/types';
-import { StopWatch } from '../../common/utils/stopWatch';
-import { sendTelemetryEvent } from '../../telemetry';
-import { Telemetry } from '../constants';
+import { createDeferred, sleep } from '../../common/utils/async';
+import { ILocalResourceUriConverter } from '../types';
 import { IWidgetScriptSourceProvider, WidgetScriptSource } from './types';
 
 // Source borrowed from https://github.com/jupyter-widgets/ipywidgets/blob/54941b7a4b54036d089652d91b39f937bde6b6cd/packages/html-manager/src/libembed-amd.ts#L33
 const unpgkUrl = 'https://unpkg.com/';
 const jsdelivrUrl = 'https://cdn.jsdelivr.net/npm/';
+
+// tslint:disable: no-var-requires no-require-imports
+const sanitize = require('sanitize-filename');
+
 function moduleNameToCDNUrl(cdn: string, moduleName: string, moduleVersion: string) {
     let packageName = moduleName;
     let fileName = 'index'; // default filename
@@ -29,6 +38,16 @@ function moduleNameToCDNUrl(cdn: string, moduleName: string, moduleVersion: stri
         fileName = moduleName.substr(index + 1);
         packageName = moduleName.substr(0, index);
     }
+    if (cdn === jsdelivrUrl) {
+        // Js Delivr doesn't support ^ in the version. It needs an exact version
+        if (moduleVersion.startsWith('^')) {
+            moduleVersion = moduleVersion.slice(1);
+        }
+        // Js Delivr also needs the .js file on the end.
+        if (!fileName.endsWith('.js')) {
+            fileName = fileName.concat('.js');
+        }
+    }
     return `${cdn}${packageName}@${moduleVersion}/dist/${fileName}`;
 }
 
@@ -52,40 +71,177 @@ export class CDNWidgetScriptSourceProvider implements IWidgetScriptSourceProvide
         const settings = this.configurationSettings.getSettings(undefined);
         return settings.datascience.widgetScriptSources;
     }
-    public static validUrls = new Map<string, boolean>();
+    private cache = new Map<string, WidgetScriptSource>();
     constructor(
         private readonly configurationSettings: IConfigurationService,
-        private readonly httpClient: IHttpClient
+        private readonly httpClient: IHttpClient,
+        private readonly localResourceUriConverter: ILocalResourceUriConverter,
+        private readonly fileSystem: IFileSystem
     ) {}
     public dispose() {
-        // Noop.
+        this.cache.clear();
     }
     public async getWidgetScriptSource(moduleName: string, moduleVersion: string): Promise<WidgetScriptSource> {
-        const cdns = [...this.cdnProviders];
-        while (cdns.length) {
-            const cdn = cdns.shift();
-            const cdnBaseUrl = getCDNPrefix(cdn);
-            if (!cdnBaseUrl || !cdn) {
-                continue;
+        // First see if we already have it downloaded.
+        const key = this.getModuleKey(moduleName, moduleVersion);
+        const diskPath = path.join(this.localResourceUriConverter.rootScriptFolder.fsPath, key, 'index.js');
+        let cached = this.cache.get(key);
+        let tempFile: TemporaryFile | undefined;
+
+        // Might be on disk, try there first.
+        if (!cached) {
+            if (diskPath && (await this.fileSystem.fileExists(diskPath))) {
+                const scriptUri = (await this.localResourceUriConverter.asWebviewUri(Uri.file(diskPath))).toString();
+                cached = { moduleName, scriptUri, source: 'cdn' };
+                this.cache.set(key, cached);
             }
-            const scriptUri = moduleNameToCDNUrl(cdnBaseUrl, moduleName, moduleVersion);
-            const exists = await this.getUrlForWidget(cdn, scriptUri);
-            if (exists) {
-                return { moduleName, scriptUri, source: 'cdn' };
+        }
+
+        // If still not found, download it.
+        if (!cached) {
+            try {
+                // Make sure the disk path directory exists. We'll be downloading it to there.
+                await this.fileSystem.createDirectory(path.dirname(diskPath));
+
+                // Then get the first one that returns.
+                tempFile = await this.downloadFastestCDN(moduleName, moduleVersion);
+                if (tempFile) {
+                    // Need to copy from the temporary file to our real file (note: VSC filesystem fails to copy so just use straight file system)
+                    await fs.copyFile(tempFile.filePath, diskPath);
+
+                    // Now we can generate the script URI so the local converter doesn't try to copy it.
+                    const scriptUri = (
+                        await this.localResourceUriConverter.asWebviewUri(Uri.file(diskPath))
+                    ).toString();
+                    cached = { moduleName, scriptUri, source: 'cdn' };
+                } else {
+                    cached = { moduleName };
+                }
+            } catch (exc) {
+                traceError('Error downloading from CDN: ', exc);
+                cached = { moduleName };
+            } finally {
+                if (tempFile) {
+                    tempFile.dispose();
+                }
+            }
+            this.cache.set(key, cached);
+        }
+
+        return cached;
+    }
+
+    private async downloadFastestCDN(moduleName: string, moduleVersion: string) {
+        const deferred = createDeferred<TemporaryFile | undefined>();
+        Promise.all(
+            // For each CDN, try to download it.
+            this.cdnProviders.map((cdn) =>
+                this.downloadFromCDN(moduleName, moduleVersion, cdn).then((t) => {
+                    // First one to get here wins. Meaning the first one that
+                    // returns a valid temporary file. If a request doesn't download it will
+                    // return undefined.
+                    if (!deferred.resolved && t) {
+                        deferred.resolve(t);
+                    }
+                })
+            )
+        )
+            .then((_a) => {
+                // If after running all requests, we're still not resolved, then return empty.
+                // This would happen if both unpkg.com and jsdelivr failed.
+                if (!deferred.resolved) {
+                    deferred.resolve(undefined);
+                }
+            })
+            .ignoreErrors();
+
+        // Note, we only wait until one download finishes. We don't need to wait
+        // for everybody (hence the use of the deferred)
+        return deferred.promise;
+    }
+
+    private async downloadFromCDN(
+        moduleName: string,
+        moduleVersion: string,
+        cdn: WidgetCDNs
+    ): Promise<TemporaryFile | undefined> {
+        // First validate CDN
+        const downloadUrl = await this.generateDownloadUri(moduleName, moduleVersion, cdn);
+        if (downloadUrl) {
+            // Then see if we can download the file.
+            try {
+                return await this.downloadFile(downloadUrl);
+            } catch (exc) {
+                // Something goes wrong, just fail
             }
         }
-        traceWarning(`Widget Script not found for ${moduleName}@${moduleVersion}`);
-        return { moduleName };
     }
-    private async getUrlForWidget(cdn: string, url: string): Promise<boolean> {
-        if (CDNWidgetScriptSourceProvider.validUrls.has(url)) {
-            return CDNWidgetScriptSourceProvider.validUrls.get(url)!;
+
+    private async generateDownloadUri(
+        moduleName: string,
+        moduleVersion: string,
+        cdn: WidgetCDNs
+    ): Promise<string | undefined> {
+        const cdnBaseUrl = getCDNPrefix(cdn);
+        if (cdnBaseUrl) {
+            return moduleNameToCDNUrl(cdnBaseUrl, moduleName, moduleVersion);
         }
+        return undefined;
+    }
+
+    private getModuleKey(moduleName: string, moduleVersion: string) {
+        return sanitize(sha256().update(`${moduleName}${moduleVersion}`).digest('hex'));
+    }
 
-        const stopWatch = new StopWatch();
-        const exists = await this.httpClient.exists(url);
-        sendTelemetryEvent(Telemetry.DiscoverIPyWidgetNamesCDNPerf, stopWatch.elapsedTime, { cdn, exists });
-        CDNWidgetScriptSourceProvider.validUrls.set(url, exists);
-        return exists;
+    private handleResponse(req: request.Request, filePath: string): Promise<boolean> {
+        const deferred = createDeferred<boolean>();
+        // tslint:disable-next-line: no-any
+        const errorHandler = (e: any) => {
+            traceError('Error downloading from CDN', e);
+            deferred.resolve(false);
+        };
+        req.on('response', (r) => {
+            if (r.statusCode === 200) {
+                const ws = this.fileSystem.createWriteStream(filePath);
+                r.on('error', errorHandler)
+                    .pipe(ws)
+                    .on('close', () => deferred.resolve(true));
+            } else if (r.statusCode === 429) {
+                // Special case busy. Sleep for 500 milliseconds
+                sleep(500)
+                    .then(() => deferred.resolve(false))
+                    .ignoreErrors();
+            } else {
+                deferred.resolve(false);
+            }
+        }).on('error', errorHandler);
+        return deferred.promise;
+    }
+
+    private async downloadFile(downloadUrl: string): Promise<TemporaryFile | undefined> {
+        // Create a temp file to download the results to
+        const tempFile = await this.fileSystem.createTemporaryFile('.js');
+
+        // Otherwise do an http get on the url. Retry at least 5 times
+        let retryCount = 5;
+        let success = false;
+        while (retryCount > 0 && !success) {
+            let req: request.Request;
+            try {
+                req = await this.httpClient.downloadFile(downloadUrl);
+                success = await this.handleResponse(req, tempFile.filePath);
+            } catch (exc) {
+                traceInfo(`Error downloading from ${downloadUrl}: `, exc);
+            } finally {
+                retryCount -= 1;
+            }
+        }
+
+        // Once we make it out, return result
+        if (success) {
+            return tempFile;
+        } else {
+            tempFile.dispose();
+        }
     }
 }

src/client/datascience/ipywidgets/ipyWidgetMessageDispatcher.ts

@@ -14,7 +14,7 @@ import { createDeferred, Deferred } from '../../common/utils/async';
 import { noop } from '../../common/utils/misc';
 import { deserializeDataViews, serializeDataViews } from '../../common/utils/serializers';
 import { sendTelemetryEvent } from '../../telemetry';
-import { Telemetry } from '../constants';
+import { Identifiers, Telemetry } from '../constants';
 import { IInteractiveWindowMapping, IPyWidgetMessages } from '../interactive-common/interactiveWindowTypes';
 import { INotebook, INotebookProvider, KernelSocketInformation } from '../types';
 import { IIPyWidgetMessageDispatcher, IPyWidgetMessage } from './types';
@@ -280,9 +280,16 @@ export class IPyWidgetMessageDispatcher implements IIPyWidgetMessageDispatcher {
                 return;
             }
 
+            traceInfo(`Registering commtarget ${targetName}`);
             this.commTargetsRegistered.add(targetName);
             this.pendingTargetNames.delete(targetName);
-            notebook.registerCommTarget(targetName, noop);
+
+            // Skip the predefined target. It should have been registered
+            // inside the kernel on startup. However we
+            // still need to track it here.
+            if (targetName !== Identifiers.DefaultCommTarget) {
+                notebook.registerCommTarget(targetName, noop);
+            }
         }
     }