Skip to content

Commit ce4f48c

Browse files
authored
Add message when widget script is not found on CDN (#11249)
For #11247 * Add message * Add tests * Fix typo
1 parent 332d3fa commit ce4f48c

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
@@ -152,6 +152,7 @@
152152
"Common.ok": "Ok",
153153
"Common.install": "Install",
154154
"Common.learnMore": "Learn more",
155+
"Common.reportThisIssue": "Report this issue",
155156
"OutputChannelNames.languageServer": "Python Language Server",
156157
"OutputChannelNames.python": "Python",
157158
"OutputChannelNames.pythonTest": "Python Test Log",
@@ -471,5 +472,6 @@
471472
"DataScience.loadClassFailedWithNoInternet": "Error loading {0}:{1}. Internet connection required for loading 3rd party widgets.",
472473
"DataScience.useCDNForWidgets": "Widgets require us to download supporting files from a 3rd party website. Click [here](https://aka.ms/PVSCIPyWidgets) for more information.",
473474
"DataScience.loadThirdPartyWidgetScriptsPostEnabled": "Please restart the Kernel when changing the setting 'python.dataScience.widgetScriptSources'.",
474-
"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})."
475+
"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}).",
476+
"DataScience.widgetScriptNotFoundOnCDNWidgetMightNotWork": "Unable to load a compatible version of the widget '{0}'. Expected behavior may be affected."
475477
}

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 {
@@ -865,6 +866,10 @@ export namespace DataScience {
865866
'DataScience.enableCDNForWidgetsSetting',
866867
"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})."
867868
);
869+
export const widgetScriptNotFoundOnCDNWidgetMightNotWork = localize(
870+
'DataScience.widgetScriptNotFoundOnCDNWidgetMightNotWork',
871+
"Unable to load a compatible version of the widget '{0}'. Expected behavior may be affected."
872+
);
868873
}
869874

870875
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
@@ -1933,6 +1933,10 @@ export interface IEventNamePropertyMapping {
19331933
* Where did we find the hashed name (CDN or user environment or remote jupyter).
19341934
*/
19351935
source?: 'cdn' | 'local' | 'remote';
1936+
/**
1937+
* Whether we searched CDN or not.
1938+
*/
1939+
cdnSearched: boolean;
19361940
};
19371941
/**
19381942
* 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
@@ -272,6 +272,47 @@ suite('Data Science - ipywidget - Widget Script Source Provider', () => {
272272
assert.deepEqual(values, { moduleName: 'module1', scriptUri: '1', source: 'cdn' });
273273
assert.isFalse(localOrRemoteSource.calledOnce);
274274
assert.isTrue(cdnSource.calledOnce);
275+
verify(appShell.showWarningMessage(anything(), anything(), anything(), anything())).never();
276+
});
277+
test('When CDN is turned on and widget script is not found, then display a warning about script not found on CDN', async () => {
278+
settings.datascience.widgetScriptSources = ['jsdelivr.com', 'unpkg.com'];
279+
const localOrRemoteSource = localLaunch
280+
? sinon.stub(LocalWidgetScriptSourceProvider.prototype, 'getWidgetScriptSource')
281+
: sinon.stub(RemoteWidgetScriptSourceProvider.prototype, 'getWidgetScriptSource');
282+
const cdnSource = sinon.stub(CDNWidgetScriptSourceProvider.prototype, 'getWidgetScriptSource');
283+
284+
localOrRemoteSource.resolves({ moduleName: 'module1' });
285+
cdnSource.resolves({ moduleName: 'module1' });
286+
287+
scriptSourceProvider.initialize();
288+
let values = await scriptSourceProvider.getWidgetScriptSource('module1', '1');
289+
290+
assert.deepEqual(values, { moduleName: 'module1' });
291+
assert.isTrue(localOrRemoteSource.calledOnce);
292+
assert.isTrue(cdnSource.calledOnce);
293+
verify(
294+
appShell.showWarningMessage(
295+
DataScience.widgetScriptNotFoundOnCDNWidgetMightNotWork().format('module1'),
296+
anything(),
297+
anything(),
298+
anything()
299+
)
300+
).once();
301+
302+
// Ensure message is not displayed more than once.
303+
values = await scriptSourceProvider.getWidgetScriptSource('module1', '1');
304+
305+
assert.deepEqual(values, { moduleName: 'module1' });
306+
assert.isTrue(localOrRemoteSource.calledTwice);
307+
assert.isTrue(cdnSource.calledTwice);
308+
verify(
309+
appShell.showWarningMessage(
310+
DataScience.widgetScriptNotFoundOnCDNWidgetMightNotWork().format('module1'),
311+
anything(),
312+
anything(),
313+
anything()
314+
)
315+
).once();
275316
});
276317
});
277318
});

0 commit comments

Comments
 (0)