Commit ⋅ aws/aws-toolkit-vscode

fix(test): telemetry settings errors

many settings errors like this in the test logs:

Failed to read key &quot;telemetry&quot;: TypeError: Failed to cast type
&quot;object&quot; to convertLegacy: TypeError: Unknown telemetry setting:
[object Object],[object Object]: Error:
    at TelemetryConfig.get (/Users/runner/work/aws-toolkit-vscode/aws-toolkit-vscode/src/shared/settings.ts<span class="emoji" style="background-image:url(https://codecov-cdn.storage.googleapis.com/v4.6.6-e00140d/media/images/emoji/193.png)"></span>27)

justinmk3 CI Passed

5199aaa master 3c7f54b

Showing 2 of 4 files from the diff.

96.36% 100.00% -0.25%

src/shared/telemetry/activation.ts changed.

89.50% 100.00% +0.48%

src/shared/settings.ts changed.

Other files ignored by Codecov

src/test/shared/telemetry/activation.test.ts has changed.

src/test/shared/settingsConfiguration.test.ts has changed.

@@ -17,7 +17,6 @@


const LEGACY_SETTINGS_TELEMETRY_VALUE_DISABLE = 'Disable'
const LEGACY_SETTINGS_TELEMETRY_VALUE_ENABLE = 'Enable'
const TELEMETRY_SETTING_DEFAULT = true

export const noticeResponseViewSettings = localize('AWS.telemetry.notificationViewSettings', 'Settings')
export const noticeResponseOk = localize('AWS.telemetry.notificationOk', 'OK')

@@ -37,7 +36,13 @@

export async function activate(extensionContext: vscode.ExtensionContext, awsContext: AwsContext, settings: Settings) {
    globals.telemetry = new DefaultTelemetryService(extensionContext, awsContext, getComputeRegion())

    const config = new TelemetryConfig(settings)
    const config = new TelemetryConfig(
        settings,
        // Disable `throwInvalid` because:
        //   1. Telemetry must never prevent normal Toolkit operation.
        //   2. For tests, bad data is intentional, so these errors add unwanted noise in the test logs.
        false
    )
    globals.telemetry.telemetryEnabled = config.isEnabled()

    extensionContext.subscriptions.push(

@@ -50,7 +55,7 @@


    // Prompt user about telemetry if they haven't been
    if (!isCloud9() && !hasUserSeenTelemetryNotice(extensionContext)) {
        showTelemetryNotice(extensionContext, config)
        showTelemetryNotice(extensionContext)
    }
}


@@ -71,17 +76,7 @@


export class TelemetryConfig extends fromExtensionManifest('aws', { telemetry: convertLegacy }) {
    public isEnabled(): boolean {
        try {
            return this.get('telemetry', TELEMETRY_SETTING_DEFAULT)
        } catch (error) {
            vscode.window.showErrorMessage(
                localize(
                    'AWS.message.error.settings.telemetry.invalid_type',
                    'The aws.telemetry value must be a boolean'
                )
            )
            return TELEMETRY_SETTING_DEFAULT
        }
        return this.get('telemetry', true)
    }
}


@@ -101,7 +96,7 @@

 * Prompts user to Enable/Disable/Defer on Telemetry, then
 * handles the response appropriately.
 */
function showTelemetryNotice(extensionContext: vscode.ExtensionContext, config: TelemetryConfig) {
function showTelemetryNotice(extensionContext: vscode.ExtensionContext) {
    getLogger().verbose('Showing telemetry notice')

    const telemetryNoticeText: string = localize(

@@ -166,18 +166,26 @@

    type Inner = FromDescriptor<T>

    // Class names are not always stable, especially when bundling
    function makeLogger(name = 'Settings') {
    function makeLogger(name = 'Settings', loglevel: 'debug' | 'error') {
        const prefix = `${isNameMangled() ? 'Settings' : name} (${section})`
        return (message: string) => getLogger().debug(`${prefix}: ${message}`)
        return (message: string) =>
            loglevel === 'debug'
                ? getLogger().debug(`${prefix}: ${message}`)
                : getLogger().error(`${prefix}: ${message}`)
    }

    return class AnonymousSettings implements TypedSettings<Inner> {
        private readonly config = this.settings.getSection(section)
        private readonly disposables: vscode.Disposable[] = []
        // TODO(sijaden): add metadata prop to `Logger` so we don't need to make one-off log functions
        protected readonly log = makeLogger(Object.getPrototypeOf(this)?.constructor?.name)
        protected readonly log = makeLogger(Object.getPrototypeOf(this)?.constructor?.name, 'debug')
        protected readonly logErr = makeLogger(Object.getPrototypeOf(this)?.constructor?.name, 'error')

        public constructor(private readonly settings: ClassToInterfaceType<Settings> = Settings.instance) {}
        public constructor(
            private readonly settings: ClassToInterfaceType<Settings> = Settings.instance,
            /** Throw an exception if the user config is invalid or the caller type doesn't match the stored type. */
            private readonly throwInvalid: boolean = isAutomation()
        ) {}

        public get onDidChange() {
            return this.getChangedEmitter().event

@@ -186,14 +194,11 @@

        public get<K extends keyof Inner>(key: K & string, defaultValue?: Inner[K]) {
            try {
                return this.getOrThrow(key, defaultValue)
            } catch (error) {
                this.log(`failed to read key "${key}": ${error}`)

                if (isAutomation() || defaultValue === undefined) {
                    throw new Error(`Failed to read key "${key}": ${error}`)
            } catch (e) {
                if (this.throwInvalid || defaultValue === undefined) {
                    throw new Error(`Failed to read key "${key}": ${e}`)
                }

                this.log(`using default value for "${key}"`)
                this.logErr(`using default for key "${key}", read failed: ${(e as Error).message ?? '?'}`)

                return defaultValue
            }

@@ -255,8 +260,13 @@


                for (const key of props.filter(isDifferent)) {
                    this.log(`key "${key}" changed`)
                    store[key] = this.get(key)
                    emitter.fire({ key })
                    try {
                        store[key] = this.get(key)
                        emitter.fire({ key })
                    } catch {
                        // getChangedEmitter() can't provide a default value so
                        // we must silence errors here. Logging is done by this.get().
                    }
                }
            })


Files		•	•	•	Coverage
src	19,337	13,285	0	6,052	68.70%
Project Totals (371 files)	19,337	13,285	0	6,052	68.70%

macos-unittests

View CI Build Download

macos-unittests

View CI Build Download

windows-unittests

View CI Build Download

macos-unittests

View CI Build Download

windows-unittests

View CI Build Download

# To validate:
#   cat codecov.yml | curl --data-binary @- https://codecov.io/validate

codecov:
    notify:
        require_ci_to_pass: no

coverage:
    precision: 2
    round: down
    range: '70...100'

    status:
        project:
            default:
                threshold: 1
                informational: true
        patch: no
        changes: no

comment: off

github_checks:
    annotations: false

Read the docs

Interact now

The inner-most circle is the entire project, moving away from the center are folders then, finally, a single file. The size and color of each slice is representing the number of statements and the coverage, respectively.

SVG

The top section represents the entire project. Proceeding with folders and finally individual files. The size and color of each slice is representing the number of statements and the coverage, respectively.

SVG

Each block represents a single file in the project. The size and color of each block is represented by the number of statements and the coverage, respectively.

SVG

Learn more about the coverage sunburst graph

71	76
72	77		export class TelemetryConfig extends fromExtensionManifest('aws', { telemetry: convertLegacy }) {
73	78		public isEnabled(): boolean {
74		-	try {
75		-	return this.get('telemetry', TELEMETRY_SETTING_DEFAULT)
76		-	} catch (error) {
77		-	vscode.window.showErrorMessage(
78		-	localize(
79		-	'AWS.message.error.settings.telemetry.invalid_type',
80		-	'The aws.telemetry value must be a boolean'
81		-	)
82		-	)
83		-	return TELEMETRY_SETTING_DEFAULT
84		-	}
	79	+	return this.get('telemetry', true)
85	80		}
86	81		}
87	82

166	166		type Inner = FromDescriptor<T>
167	167
168	168		// Class names are not always stable, especially when bundling
169		-	function makeLogger(name = 'Settings') {
	169	+	function makeLogger(name = 'Settings', loglevel: 'debug' \| 'error') {
170	170		const prefix = `${isNameMangled() ? 'Settings' : name} (${section})`
171		-	return (message: string) => getLogger().debug(`${prefix}: ${message}`)
	171	+	return (message: string) =>
	172	+	loglevel === 'debug'
	173	+	? getLogger().debug(`${prefix}: ${message}`)
	174	+	: getLogger().error(`${prefix}: ${message}`)
172	175		}
173	176
174	177		return class AnonymousSettings implements TypedSettings<Inner> {
175	178		private readonly config = this.settings.getSection(section)
176	179		private readonly disposables: vscode.Disposable[] = []
177	180		// TODO(sijaden): add metadata prop to `Logger` so we don't need to make one-off log functions
178		-	protected readonly log = makeLogger(Object.getPrototypeOf(this)?.constructor?.name)
	181	+	protected readonly log = makeLogger(Object.getPrototypeOf(this)?.constructor?.name, 'debug')
	182	+	protected readonly logErr = makeLogger(Object.getPrototypeOf(this)?.constructor?.name, 'error')
179	183
180		-	public constructor(private readonly settings: ClassToInterfaceType<Settings> = Settings.instance) {}
	184	+	public constructor(
	185	+	private readonly settings: ClassToInterfaceType<Settings> = Settings.instance,
	186	+	/** Throw an exception if the user config is invalid or the caller type doesn't match the stored type. */
	187	+	private readonly throwInvalid: boolean = isAutomation()
	188	+	) {}
181	189
182	190		public get onDidChange() {
183	191		return this.getChangedEmitter().event

255	260
256	261		for (const key of props.filter(isDifferent)) {
257	262		this.log(`key "${key}" changed`)
258		-	store[key] = this.get(key)
259		-	emitter.fire({ key })
	263	+	try {
	264	+	store[key] = this.get(key)
	265	+	emitter.fire({ key })
	266	+	} catch {
	267	+	// getChangedEmitter() can't provide a default value so
	268	+	// we must silence errors here. Logging is done by this.get().
	269	+	}
260	270		}
261	271		})
262	272

Navigation	Overlay
`t` Navigate files	`h` Toggle hits
`y` Change url to tip of branch	`m` Toggle misses
`b / v` Jump to prev/next hit line	`p` Toggle partial
`z / x` Jump to prev/next missed or partial line	`1..9` Toggle flags
`shift + o` Open current page in GitHub	`a` Toggle all on
`/ or ?` Show keyboard shortcuts dialog	`c` Toggle context lines or commits

17	17
18	18		const LEGACY_SETTINGS_TELEMETRY_VALUE_DISABLE = 'Disable'
19	19		const LEGACY_SETTINGS_TELEMETRY_VALUE_ENABLE = 'Enable'
20		-	const TELEMETRY_SETTING_DEFAULT = true
21	20
22	21		export const noticeResponseViewSettings = localize('AWS.telemetry.notificationViewSettings', 'Settings')
23	22		export const noticeResponseOk = localize('AWS.telemetry.notificationOk', 'OK')

37	36		export async function activate(extensionContext: vscode.ExtensionContext, awsContext: AwsContext, settings: Settings) {
38	37		globals.telemetry = new DefaultTelemetryService(extensionContext, awsContext, getComputeRegion())
39	38
40		-	const config = new TelemetryConfig(settings)
	39	+	const config = new TelemetryConfig(
	40	+	settings,
	41	+	// Disable `throwInvalid` because:
	42	+	// 1. Telemetry must never prevent normal Toolkit operation.
	43	+	// 2. For tests, bad data is intentional, so these errors add unwanted noise in the test logs.
	44	+	false
	45	+	)
41	46		globals.telemetry.telemetryEnabled = config.isEnabled()
42	47
43	48		extensionContext.subscriptions.push(

50	55
51	56		// Prompt user about telemetry if they haven't been
52	57		if (!isCloud9() && !hasUserSeenTelemetryNotice(extensionContext)) {
53		-	showTelemetryNotice(extensionContext, config)
	58	+	showTelemetryNotice(extensionContext)
54	59		}
55	60		}
56	61

101	96		* Prompts user to Enable/Disable/Defer on Telemetry, then
102	97		* handles the response appropriately.
103	98		*/
104		-	function showTelemetryNotice(extensionContext: vscode.ExtensionContext, config: TelemetryConfig) {
	99	+	function showTelemetryNotice(extensionContext: vscode.ExtensionContext) {
105	100		getLogger().verbose('Showing telemetry notice')
106	101
107	102		const telemetryNoticeText: string = localize(

186	194		public get<K extends keyof Inner>(key: K & string, defaultValue?: Inner[K]) {
187	195		try {
188	196		return this.getOrThrow(key, defaultValue)
189		-	} catch (error) {
190		-	this.log(`failed to read key "${key}": ${error}`)
191		-
192		-	if (isAutomation() \|\| defaultValue === undefined) {
193		-	throw new Error(`Failed to read key "${key}": ${error}`)
	197	+	} catch (e) {
	198	+	if (this.throwInvalid \|\| defaultValue === undefined) {
	199	+	throw new Error(`Failed to read key "${key}": ${e}`)
194	200		}
195		-
196		-	this.log(`using default value for "${key}"`)
	201	+	this.logErr(`using default for key "${key}", read failed: ${(e as Error).message ?? '?'}`)
197	202
198	203		return defaultValue
199	204		}

1	# To validate:
2	# cat codecov.yml \| curl --data-binary @- https://codecov.io/validate
3
4	codecov:
5	notify:
6	require_ci_to_pass: no
7
8	coverage:
9	precision: 2
10	round: down
11	range: '70...100'
12
13	status:
14	project:
15	default:
16	threshold: 1
17	informational: true
18	patch: no
19	changes: no
20
21	comment: off
22
23	github_checks:
24	annotations: false

Updating our web app