Skip to content

Commit 9e29be4

Browse files
committed
Add message when widget script is not found on CDN (microsoft#11249)
For #11247 * Add message * Add tests * Fix typo
1 parent 9dd4b7a commit 9e29be4

File tree

5 files changed

+94
-3
lines changed

5 files changed

+94
-3
lines changed

package.nls.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@
148148
"Common.ok": "Ok",
149149
"Common.install": "Install",
150150
"Common.learnMore": "Learn more",
151+
"Common.reportThisIssue": "Report this issue",
151152
"OutputChannelNames.languageServer": "Python Language Server",
152153
"OutputChannelNames.python": "Python",
153154
"OutputChannelNames.pythonTest": "Python Test Log",
@@ -467,5 +468,6 @@
467468
"DataScience.loadClassFailedWithNoInternet": "Error loading {0}:{1}. Internet connection required for loading 3rd party widgets.",
468469
"DataScience.useCDNForWidgets": "Widgets require us to download supporting files from a 3rd party website. Click [here](https://aka.ms/PVSCIPyWidgets) for more information.",
469470
"DataScience.loadThirdPartyWidgetScriptsPostEnabled": "Please restart the Kernel when changing the setting 'python.dataScience.widgetScriptSources'.",
470-
"DataScience.enableCDNForWidgetsSetting": "Widgets require us to download supporting files from a 3rd party website. Click <a href='https://command:python.datascience.enableLoadingWidgetScriptsFromThirdPartySource'>here</a> to enable this or click <a href='https://aka.ms/PVSCIPyWidgets'>here</a> for more information. (Error loading {0}:{1})."
471+
"DataScience.enableCDNForWidgetsSetting": "Widgets require us to download supporting files from a 3rd party website. Click <a href='https://command:python.datascience.enableLoadingWidgetScriptsFromThirdPartySource'>here</a> to enable this or click <a href='https://aka.ms/PVSCIPyWidgets'>here</a> for more information. (Error loading {0}:{1}).",
472+
"DataScience.widgetScriptNotFoundOnCDNWidgetMightNotWork": "Unable to load a compatible version of the widget '{0}'. Expected behavior may be affected."
471473
}

src/client/common/utils/localize.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ export namespace Common {
7575
export const moreInfo = localize('Common.moreInfo', 'More Info');
7676
export const learnMore = localize('Common.learnMore', 'Learn more');
7777
export const and = localize('Common.and', 'and');
78+
export const reportThisIssue = localize('Common.reportThisIssue', 'Report this issue');
7879
}
7980

8081
export namespace AttachProcess {
@@ -849,6 +850,10 @@ export namespace DataScience {
849850
'DataScience.enableCDNForWidgetsSetting',
850851
"Widgets require us to download supporting files from a 3rd party website. Click <a href='https://command:python.datascience.enableLoadingWidgetScriptsFromThirdPartySource'>here</a> to enable this or click <a href='https://aka.ms/PVSCIPyWidgets'>here</a> for more information. (Error loading {0}:{1})."
851852
);
853+
export const widgetScriptNotFoundOnCDNWidgetMightNotWork = localize(
854+
'DataScience.widgetScriptNotFoundOnCDNWidgetMightNotWork',
855+
"Unable to load a compatible version of the widget '{0}'. Expected behavior may be affected."
856+
);
852857
}
853858

854859
export namespace DebugConfigStrings {

src/client/datascience/ipywidgets/ipyWidgetScriptSourceProvider.ts

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import { sha256 } from 'hash.js';
77
import { ConfigurationChangeEvent, ConfigurationTarget } from 'vscode';
88
import { IApplicationShell, IWorkspaceService } from '../../common/application/types';
9+
import '../../common/extensions';
910
import { traceError } from '../../common/logger';
1011
import { IFileSystem } from '../../common/platform/types';
1112
import {
@@ -17,6 +18,7 @@ import {
1718
} from '../../common/types';
1819
import { createDeferred, Deferred } from '../../common/utils/async';
1920
import { Common, DataScience } from '../../common/utils/localize';
21+
import { noop } from '../../common/utils/misc';
2022
import { IInterpreterService } from '../../interpreter/contracts';
2123
import { sendTelemetryEvent } from '../../telemetry';
2224
import { Telemetry } from '../constants';
@@ -27,6 +29,7 @@ import { RemoteWidgetScriptSourceProvider } from './remoteWidgetScriptSourceProv
2729
import { IWidgetScriptSourceProvider, WidgetScriptSource } from './types';
2830

2931
const GlobalStateKeyToTrackIfUserConfiguredCDNAtLeastOnce = 'IPYWidgetCDNConfigured';
32+
const GlobalStateKeyToNeverWarnAboutScriptsNotFoundOnCDN = 'IPYWidgetNotFoundOnCDN';
3033

3134
/**
3235
* This class decides where to get widget scripts from.
@@ -35,13 +38,15 @@ const GlobalStateKeyToTrackIfUserConfiguredCDNAtLeastOnce = 'IPYWidgetCDNConfigu
3538
* If user has not configured antying, user will be presented with a prompt.
3639
*/
3740
export class IPyWidgetScriptSourceProvider implements IWidgetScriptSourceProvider {
41+
private readonly notifiedUserAboutWidgetScriptNotFound = new Set<string>();
3842
private scriptProviders?: IWidgetScriptSourceProvider[];
3943
private configurationPromise?: Deferred<void>;
4044
private get configuredScriptSources(): readonly WidgetCDNs[] {
4145
const settings = this.configurationSettings.getSettings(undefined);
4246
return settings.datascience.widgetScriptSources;
4347
}
4448
private readonly userConfiguredCDNAtLeastOnce: IPersistentState<boolean>;
49+
private readonly neverWarnAboutScriptsNotFoundOnCDN: IPersistentState<boolean>;
4550
constructor(
4651
private readonly notebook: INotebook,
4752
private readonly localResourceUriConverter: ILocalResourceUriConverter,
@@ -57,6 +62,10 @@ export class IPyWidgetScriptSourceProvider implements IWidgetScriptSourceProvide
5762
GlobalStateKeyToTrackIfUserConfiguredCDNAtLeastOnce,
5863
false
5964
);
65+
this.neverWarnAboutScriptsNotFoundOnCDN = this.stateFactory.createGlobalPersistentState<boolean>(
66+
GlobalStateKeyToNeverWarnAboutScriptsNotFoundOnCDN,
67+
false
68+
);
6069
}
6170
public initialize() {
6271
this.workspaceService.onDidChangeConfiguration(this.onSettingsChagned.bind(this));
@@ -94,16 +103,46 @@ export class IPyWidgetScriptSourceProvider implements IWidgetScriptSourceProvide
94103

95104
sendTelemetryEvent(Telemetry.HashedIPyWidgetNameUsed, undefined, {
96105
hashedName: sha256().update(found.moduleName).digest('hex'),
97-
source: found.source
106+
source: found.source,
107+
cdnSearched: this.configuredScriptSources.length > 0
98108
});
99109

100110
if (!found.scriptUri) {
101111
traceError(`Script source for Widget ${moduleName}@${moduleVersion} not found`);
102112
}
113+
this.handleWidgetSourceNotFoundOnCDN(found).ignoreErrors();
103114
return found;
104115
}
116+
private async handleWidgetSourceNotFoundOnCDN(widgetSource: WidgetScriptSource) {
117+
// if widget exists nothing to do.
118+
if (widgetSource.source === 'cdn' || this.neverWarnAboutScriptsNotFoundOnCDN.value === true) {
119+
return;
120+
}
121+
if (
122+
this.notifiedUserAboutWidgetScriptNotFound.has(widgetSource.moduleName) ||
123+
this.configuredScriptSources.length === 0
124+
) {
125+
return;
126+
}
127+
this.notifiedUserAboutWidgetScriptNotFound.add(widgetSource.moduleName);
128+
const selection = await this.appShell.showWarningMessage(
129+
DataScience.widgetScriptNotFoundOnCDNWidgetMightNotWork().format(widgetSource.moduleName),
130+
Common.ok(),
131+
Common.doNotShowAgain(),
132+
Common.reportThisIssue()
133+
);
134+
switch (selection) {
135+
case Common.doNotShowAgain():
136+
return this.neverWarnAboutScriptsNotFoundOnCDN.updateValue(true);
137+
case Common.reportThisIssue():
138+
return this.appShell.openUrl('https://aka.ms/CreatePVSCDataScienceIssue');
139+
default:
140+
noop();
141+
}
142+
}
143+
105144
private onSettingsChagned(e: ConfigurationChangeEvent) {
106-
if (e.affectsConfiguration('dataScience.widgetScriptSources')) {
145+
if (e.affectsConfiguration('python.dataScience.widgetScriptSources')) {
107146
this.rebuildProviders();
108147
}
109148
}

src/client/telemetry/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1951,6 +1951,10 @@ export interface IEventNamePropertyMapping {
19511951
* Where did we find the hashed name (CDN or user environment or remote jupyter).
19521952
*/
19531953
source?: 'cdn' | 'local' | 'remote';
1954+
/**
1955+
* Whether we searched CDN or not.
1956+
*/
1957+
cdnSearched: boolean;
19541958
};
19551959
/**
19561960
* Telemetry event sent with name of a Widget found.

src/test/datascience/ipywidgets/ipyWidgetScriptSourceProvider.unit.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,47 @@ suite('xxxData Science - ipywidget - Widget Script Source Provider', () => {
269269
assert.deepEqual(values, { moduleName: 'module1', scriptUri: '1', source: 'cdn' });
270270
assert.isFalse(localOrRemoteSource.calledOnce);
271271
assert.isTrue(cdnSource.calledOnce);
272+
verify(appShell.showWarningMessage(anything(), anything(), anything(), anything())).never();
273+
});
274+
test('When CDN is turned on and widget script is not found, then display a warning about script not found on CDN', async () => {
275+
settings.datascience.widgetScriptSources = ['jsdelivr.com', 'unpkg.com'];
276+
const localOrRemoteSource = localLaunch
277+
? sinon.stub(LocalWidgetScriptSourceProvider.prototype, 'getWidgetScriptSource')
278+
: sinon.stub(RemoteWidgetScriptSourceProvider.prototype, 'getWidgetScriptSource');
279+
const cdnSource = sinon.stub(CDNWidgetScriptSourceProvider.prototype, 'getWidgetScriptSource');
280+
281+
localOrRemoteSource.resolves({ moduleName: 'module1' });
282+
cdnSource.resolves({ moduleName: 'module1' });
283+
284+
scriptSourceProvider.initialize();
285+
let values = await scriptSourceProvider.getWidgetScriptSource('module1', '1');
286+
287+
assert.deepEqual(values, { moduleName: 'module1' });
288+
assert.isTrue(localOrRemoteSource.calledOnce);
289+
assert.isTrue(cdnSource.calledOnce);
290+
verify(
291+
appShell.showWarningMessage(
292+
DataScience.widgetScriptNotFoundOnCDNWidgetMightNotWork().format('module1'),
293+
anything(),
294+
anything(),
295+
anything()
296+
)
297+
).once();
298+
299+
// Ensure message is not displayed more than once.
300+
values = await scriptSourceProvider.getWidgetScriptSource('module1', '1');
301+
302+
assert.deepEqual(values, { moduleName: 'module1' });
303+
assert.isTrue(localOrRemoteSource.calledTwice);
304+
assert.isTrue(cdnSource.calledTwice);
305+
verify(
306+
appShell.showWarningMessage(
307+
DataScience.widgetScriptNotFoundOnCDNWidgetMightNotWork().format('module1'),
308+
anything(),
309+
anything(),
310+
anything()
311+
)
312+
).once();
272313
});
273314
});
274315
});

0 commit comments

Comments
 (0)