fix: reject with proper Error on audio error event (fixes #339115)

[VS Code PR Bot (vscodebot-pr)]

Summary

An accessibility signal sound fails to play, and the error event handler in doPlayAudio rejects the promise with e.error — but a DOM media error event is a plain Event that has no error property, so the promise rejects with undefined. That undefined then reaches the catch (e) block in playSound, which reads e.message and throws TypeError: Cannot read properties of undefined (reading 'message'). Impact: an unhandled error is logged to telemetry whenever an accessibility cue sound fails to load/play (e.g. missing/blocked media), affecting 425 users on stable 1.140.0.

Fixes #339115
Recommended reviewer: @meganrogge

Culprit Commit

Field	Value
Commit	pre-existing — not a regression
Author	n/a
PR	n/a
Message	n/a
Why	The faulty reject(e.error) has existed since the file was created; no recent commit changed the triggering logic. This bucket is new on the 1.140.0 stable channel but the defect is long-standing (surfaced at stable scale / under conditions where a cue sound fails to play). This is best characterized as a pre-existing bug newly observed, not a code regression.

No culprit commit identified: the defective line predates the available
history for this file and no commit in the regression window altered the
audio error-handling path. The fix targets the long-standing producer bug.

Code Flow

sequenceDiagram
    participant Caller as playSignal
    participant Sound as playSound
    participant Audio as doPlayAudio
    participant DOM as HTMLAudioElement

    Caller->>Sound: await playSound(sound)
    Sound->>Audio: await playAudio(url, volume)
    Audio->>DOM: new Audio(url)#59; audio.play()
    DOM-->>Audio: 'error' event (plain Event, no .error)
    Note over Audio: ⚠️ Root cause (L297):<br/>reject(e.error) → rejects with undefined
    Audio-->>Sound: promise rejects with undefined
    Note over Sound: 💥 L211: e.message<br/>Cannot read properties of<br/>undefined (reading 'message')

Loading

Affected Files

File	Role	Evidence
src/vs/platform/accessibilitySignal/browser/accessibilitySignalService.ts	root cause (producer)	L297: reject(e.error) — the media error event is a plain Event with no .error property, so the promise rejects with undefined
src/vs/platform/accessibilitySignal/browser/accessibilitySignalService.ts	crash site	L211 (from stack): if (!e.message.includes(...)) reads .message on the undefined rejection value

Repro Steps

1. Trigger any accessibility signal that plays a sound (e.g. enable sounds via accessibility.signalOptions, then perform an action that plays a cue).
2. Cause the audio element to emit an error event — e.g. the media resource fails to load, is blocked, or is otherwise unplayable (the browser/Electron fires error rather than resolving play()).
3. The error handler rejects the play promise with undefined; the catch (e) in playSound reads e.message and throws TypeError: Cannot read properties of undefined (reading 'message'), which is reported as an unhandled error.

Because play()'s own rejection path (L300) rejects with a real Error, the crash only manifests via the media error event path, which explains the intermittent, environment-dependent occurrence.

How the Fix Works

Chosen approach (accessibilitySignalService.ts → doPlayAudio): fix the data producer, not the crash site. The error-event handler no longer rejects with the non-existent e.error. Instead it constructs a proper Error, enriched with the real failure details from audio.error (an HTMLMediaElement's MediaError, carrying .code and .message) when available, and a generic "Failed to play audio" message otherwise. After this change the promise always rejects with a genuine Error instance, so the downstream catch (e) in playSound can always read e.message safely — the TypeError can no longer be produced.

This keeps the existing logService/console.error telemetry path intact (the catch block still runs and still logs genuine failures), and it enriches the error with cross-boundary diagnostic context (MediaError.code) rather than swallowing it.

Alternatives considered:

• Guard at the crash site (e?.message or if (!e) return in playSound's catch) — rejected: that is a crash-site guard that masks the producer bug, leaves every other consumer of playAudio receiving a meaningless undefined rejection, and discards the real failure cause.
• Wrap the play call in try/catch that swallows the error — rejected: it would hide genuine audio failures from the telemetry pipeline.

Recommended Owner

@meganrogge — area owner of accessibilitySignalService.ts; authored the large majority of this file's history and is highly active in microsoft/vscode (hundreds of commits in the last 90 days). The culprit is pre-existing, so ownership falls to the feature-area owner.

Authored with Copilot

[Copilot]

Copilot review overview

🟢 Approval recommended

The functional fix is correct; only a minor comment-format issue remains.

Review effort: Balanced
Findings: 1

Open (1)

• Inline comment exceeds one-line limit · New

What changed in this PR

Fixes accessibility signal failures rejecting with undefined, preventing a secondary TypeError.

Changes:

• Rejects audio errors with a proper Error.
• Includes available MediaError diagnostics.

File	Description
src/​vs/​platform/​accessibilitySignal/​browser/​accessibilitySignalService.ts	Corrects audio error-event rejection handling.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Bryan Chen (bryanchen-d)]

Separated the infrastructure fix from this accessibility PR as requested.

The shared workflow is restored byte-for-byte, and this PR's net diff now contains only accessibilitySignalService.ts. The ESLint scheduling change is tracked independently in #339153, with failure evidence and its CI/runtime trade-off documented there.

This is not specific to the audio change: the same ESLint exit-137 signature was verified on #335978, #339105, and #339110. The latter two subsequently passed and merged.

Fresh CI is running against this PR's restored accessibility-only head. No PR has been merged as part of this work.

Co-authored with Copilot