Skip to content

Commit 6d2a98b

Browse files
katspaughclaude
andauthored
Fix: Additional bugs and code quality improvements (#4210)
* Fix: Log unhandled async error in constructor The constructor's initial load() call silently swallowed errors with .catch(() => null), making debugging difficult when initial load fails. Issues fixed: - Added console.error to log the error for debugging - Error event is still emitted inside load() as documented - Developers can now see initial load failures in console - No change to error event behavior While errors cannot be caught from a constructor call, this provides visibility into initial load failures for debugging purposes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix: RecordPlugin blob URL memory leak The RecordPlugin created blob URLs with URL.createObjectURL() but never revoked them, causing memory leaks during multiple recordings. Issues fixed: - Added recordedBlobUrl property to track created blob URLs - Revoke previous blob URL before creating new one - Revoke blob URL in destroy() to free memory - Prevents blob accumulation during multiple recording sessions Each recording now properly cleans up its blob URL before creating a new one, preventing memory leaks from unreleased blob references. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix: Clear ResizeObserver reference after disconnect The Renderer's destroy() method disconnected the ResizeObserver but didn't clear the reference, causing minor memory retention. Issues fixed: - Set resizeObserver to null after disconnect() - Ensures reference is cleared for garbage collection - Prevents disconnected observer from remaining in memory While the observer was disconnected, the reference prevented it from being garbage collected. Now properly releases the reference. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix: Clear debounce timeout on destroy in drag handler The drag handler had a pending setTimeout that wasn't cleared when WaveSurfer was destroyed, potentially causing seekTo() calls on destroyed instances. Issues fixed: - Store unsubscribeDrag separately from subscriptions array - Add cleanup function that clears timeout and unsubscribes - Prevents setTimeout callback from executing after destroy - Avoids potential null reference errors The debounce timeout is now properly cleared when the instance is destroyed, preventing orphaned timeout callbacks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix: Add numberOfChannels bounds check in regions The region element initialization could divide by zero if numberOfChannels was 0, resulting in Infinity for elementHeight. Issues fixed: - Added check for numberOfChannels > 0 before division - Prevents Infinity from being set as CSS height value - Falls back to default 100% height for invalid channel counts - Ensures region rendering doesn't break with edge case data The region now safely handles cases where numberOfChannels is 0 or undefined, preventing invalid CSS values. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix: Handle empty color array with default value The convertColorValues() method returned empty string for empty arrays, which could cause rendering issues. Now returns default color. Issues fixed: - Added explicit check for empty color array (length === 0) - Returns default waveColor '#999' instead of empty string - Ensures waveform always has a valid color value - Prevents potential rendering failures with invalid colors Empty color arrays now fall back to a sensible default rather than an empty string that could break rendering. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix: Close previous AudioContext in RecordPlugin startMic The startMic() method created new AudioContext instances without closing previous ones when called multiple times, causing resource exhaustion. Issues fixed: - Added check for existing micStream before starting new one - Call stopMic() to clean up previous AudioContext - Prevents AudioContext accumulation from multiple startMic() calls - Ensures system audio resources are properly released Each startMic() call now properly closes the previous AudioContext before creating a new one, preventing audio resource exhaustion. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix: Remove event listeners from detached region content The setContent() method removed old content elements without removing their event listeners first, causing memory leaks from orphaned listeners on detached DOM nodes. Issues fixed: - Store content event listeners in properties for later removal - Remove listeners from old content before detaching it - Re-add listeners to new content after setting it - Prevents memory leaks from orphaned event listeners Event listeners on region content are now properly cleaned up when content is changed, preventing memory leaks from detached DOM nodes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix: Add iteration limit to prevent infinite recursion in Fetcher The watchProgress() recursive read() function had no safeguards against malformed streams that never signal completion, potentially causing infinite recursion. Issues fixed: - Added maxIterations limit (100,000) to prevent infinite loops - Added iteration counter to track read attempts - Logs error and exits if limit is reached - Protects against malformed or never-ending streams While reader.read() should eventually return done: true, this provides a safety net for edge cases with malformed streams or protocol errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0fba2ad commit 6d2a98b

File tree

5 files changed

+86
-32
lines changed

5 files changed

+86
-32
lines changed

src/fetcher.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ async function watchProgress(response: Response, progressCallback: (percentage:
44

55
const contentLength = Number(response.headers.get('Content-Length')) || 0
66
let receivedLength = 0
7+
const maxIterations = 100000 // Safety limit to prevent infinite loops
8+
let iterations = 0
79

810
// Process the data
911
const processChunk = async (value: Uint8Array | undefined) => {
@@ -14,6 +16,12 @@ async function watchProgress(response: Response, progressCallback: (percentage:
1416
}
1517

1618
const read = async () => {
19+
// Safety check to prevent infinite recursion
20+
if (iterations++ > maxIterations) {
21+
console.error('Fetcher: Maximum iterations reached, stopping read loop')
22+
return
23+
}
24+
1725
let data
1826
try {
1927
data = await reader.read()

src/plugins/record.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ class RecordPlugin extends BasePlugin<RecordPluginEvents, RecordPluginOptions> {
6767
private micStream: MicStream | null = null
6868
private unsubscribeDestroy?: () => void
6969
private unsubscribeRecordEnd?: () => void
70+
private recordedBlobUrl: string | null = null
7071

7172
/** Create an instance of the Record plugin */
7273
constructor(options: RecordPluginOptions) {
@@ -213,6 +214,11 @@ class RecordPlugin extends BasePlugin<RecordPluginEvents, RecordPluginOptions> {
213214

214215
/** Request access to the microphone and start monitoring incoming audio */
215216
public async startMic(options?: RecordPluginDeviceOptions): Promise<MediaStream> {
217+
// Stop previous mic stream if exists to clean up AudioContext
218+
if (this.micStream) {
219+
this.stopMic()
220+
}
221+
216222
let stream: MediaStream
217223
try {
218224
stream = await navigator.mediaDevices.getUserMedia({
@@ -272,7 +278,12 @@ class RecordPlugin extends BasePlugin<RecordPluginEvents, RecordPluginOptions> {
272278
this.emit(ev, blob)
273279
if (this.options.renderRecordedAudio) {
274280
this.applyOriginalOptionsIfNeeded()
275-
this.wavesurfer?.load(URL.createObjectURL(blob))
281+
// Revoke previous blob URL before creating a new one
282+
if (this.recordedBlobUrl) {
283+
URL.revokeObjectURL(this.recordedBlobUrl)
284+
}
285+
this.recordedBlobUrl = URL.createObjectURL(blob)
286+
this.wavesurfer?.load(this.recordedBlobUrl)
276287
}
277288
}
278289

@@ -355,6 +366,11 @@ class RecordPlugin extends BasePlugin<RecordPluginEvents, RecordPluginOptions> {
355366
super.destroy()
356367
this.stopRecording()
357368
this.stopMic()
369+
// Revoke blob URL to free memory
370+
if (this.recordedBlobUrl) {
371+
URL.revokeObjectURL(this.recordedBlobUrl)
372+
this.recordedBlobUrl = null
373+
}
358374
}
359375

360376
private applyOriginalOptionsIfNeeded() {

src/plugins/regions.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ class SingleRegion extends EventEmitter<RegionEvents> implements Region {
102102
public subscriptions: (() => void)[] = []
103103
public updatingSide?: UpdateSide = undefined
104104
private isRemoved = false
105+
private contentClickListener?: (e: MouseEvent) => void
106+
private contentBlurListener?: () => void
105107

106108
constructor(
107109
params: RegionParams,
@@ -218,7 +220,7 @@ class SingleRegion extends EventEmitter<RegionEvents> implements Region {
218220
let elementTop = 0
219221
let elementHeight = 100
220222

221-
if (this.channelIdx >= 0 && this.channelIdx < this.numberOfChannels) {
223+
if (this.channelIdx >= 0 && this.numberOfChannels > 0 && this.channelIdx < this.numberOfChannels) {
222224
elementHeight = 100 / this.numberOfChannels
223225
elementTop = elementHeight * this.channelIdx
224226
}
@@ -284,8 +286,10 @@ class SingleRegion extends EventEmitter<RegionEvents> implements Region {
284286
)
285287

286288
if (this.contentEditable && this.content) {
287-
this.content.addEventListener('click', (e) => this.onContentClick(e))
288-
this.content.addEventListener('blur', () => this.onContentBlur())
289+
this.contentClickListener = (e) => this.onContentClick(e)
290+
this.contentBlurListener = () => this.onContentBlur()
291+
this.content.addEventListener('click', this.contentClickListener)
292+
this.content.addEventListener('blur', this.contentBlurListener)
289293
}
290294
}
291295

@@ -375,6 +379,16 @@ class SingleRegion extends EventEmitter<RegionEvents> implements Region {
375379
public setContent(content: RegionParams['content']) {
376380
if (!this.element) return
377381

382+
// Remove event listeners from old content before removing it
383+
if (this.content && this.contentEditable) {
384+
if (this.contentClickListener) {
385+
this.content.removeEventListener('click', this.contentClickListener)
386+
}
387+
if (this.contentBlurListener) {
388+
this.content.removeEventListener('blur', this.contentBlurListener)
389+
}
390+
}
391+
378392
this.content?.remove()
379393
if (!content) {
380394
this.content = undefined
@@ -394,6 +408,11 @@ class SingleRegion extends EventEmitter<RegionEvents> implements Region {
394408
}
395409
if (this.contentEditable) {
396410
this.content.contentEditable = 'true'
411+
// Re-add event listeners to new content
412+
this.contentClickListener = (e) => this.onContentClick(e)
413+
this.contentBlurListener = () => this.onContentBlur()
414+
this.content.addEventListener('click', this.contentClickListener)
415+
this.content.addEventListener('blur', this.contentBlurListener)
397416
}
398417
this.content.setAttribute('part', 'region-content')
399418
this.element.appendChild(this.content)

src/renderer.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,10 @@ class Renderer extends EventEmitter<RendererEvents> {
297297
destroy() {
298298
this.subscriptions.forEach((unsubscribe) => unsubscribe())
299299
this.container.remove()
300-
this.resizeObserver?.disconnect()
300+
if (this.resizeObserver) {
301+
this.resizeObserver.disconnect()
302+
this.resizeObserver = null
303+
}
301304
this.unsubscribeOnScroll?.forEach((unsubscribe) => unsubscribe())
302305
this.unsubscribeOnScroll = []
303306
}
@@ -329,6 +332,7 @@ class Renderer extends EventEmitter<RendererEvents> {
329332
// Convert array of color values to linear gradient
330333
private convertColorValues(color?: WaveSurferOptions['waveColor']): string | CanvasGradient {
331334
if (!Array.isArray(color)) return color || ''
335+
if (color.length === 0) return '#999' // Return default color for empty array
332336
if (color.length < 2) return color[0] || ''
333337

334338
const canvasElement = document.createElement('canvas')

src/wavesurfer.ts

Lines changed: 34 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,10 @@ class WaveSurfer extends Player<WaveSurferEvents> {
200200
if (initialUrl || (peaks && duration)) {
201201
// Swallow async errors because they cannot be caught from a constructor call.
202202
// Subscribe to the wavesurfer's error event to handle them.
203-
this.load(initialUrl, peaks, duration).catch(() => null)
203+
this.load(initialUrl, peaks, duration).catch((err) => {
204+
// Log error for debugging while still emitting error event
205+
console.error('WaveSurfer initial load error:', err)
206+
})
204207
}
205208
})
206209
}
@@ -318,34 +321,38 @@ class WaveSurfer extends Player<WaveSurferEvents> {
318321

319322
// Drag
320323
{
321-
let debounce: ReturnType<typeof setTimeout>
322-
this.subscriptions.push(
323-
this.renderer.on('drag', (relativeX) => {
324-
if (!this.options.interact) return
325-
326-
// Update the visual position
327-
this.renderer.renderProgress(relativeX)
328-
329-
// Set the audio position with a debounce
330-
clearTimeout(debounce)
331-
let debounceTime
332-
333-
if (this.isPlaying()) {
334-
debounceTime = 0
335-
} else if (this.options.dragToSeek === true) {
336-
debounceTime = 200
337-
} else if (typeof this.options.dragToSeek === 'object' && this.options.dragToSeek !== undefined) {
338-
debounceTime = this.options.dragToSeek['debounceTime']
339-
}
324+
let debounce: ReturnType<typeof setTimeout> | undefined
325+
const unsubscribeDrag = this.renderer.on('drag', (relativeX) => {
326+
if (!this.options.interact) return
327+
328+
// Update the visual position
329+
this.renderer.renderProgress(relativeX)
330+
331+
// Set the audio position with a debounce
332+
clearTimeout(debounce)
333+
let debounceTime
334+
335+
if (this.isPlaying()) {
336+
debounceTime = 0
337+
} else if (this.options.dragToSeek === true) {
338+
debounceTime = 200
339+
} else if (typeof this.options.dragToSeek === 'object' && this.options.dragToSeek !== undefined) {
340+
debounceTime = this.options.dragToSeek['debounceTime']
341+
}
340342

341-
debounce = setTimeout(() => {
342-
this.seekTo(relativeX)
343-
}, debounceTime)
343+
debounce = setTimeout(() => {
344+
this.seekTo(relativeX)
345+
}, debounceTime)
344346

345-
this.emit('interaction', relativeX * this.getDuration())
346-
this.emit('drag', relativeX)
347-
}),
348-
)
347+
this.emit('interaction', relativeX * this.getDuration())
348+
this.emit('drag', relativeX)
349+
})
350+
351+
// Clear debounce timeout on destroy
352+
this.subscriptions.push(() => {
353+
clearTimeout(debounce)
354+
unsubscribeDrag()
355+
})
349356
}
350357
}
351358

0 commit comments

Comments
 (0)