Skip to content

fix: Clean up BatchUploader timers and event listeners on re-init#1251

Merged
rmi22186 merged 3 commits intodevelopmentfrom
fix/batch-uploader-timer-leak
Apr 17, 2026
Merged

fix: Clean up BatchUploader timers and event listeners on re-init#1251
rmi22186 merged 3 commits intodevelopmentfrom
fix/batch-uploader-timer-leak

Conversation

@rmi22186
Copy link
Copy Markdown
Member

Summary

  • Fix resource leaks in BatchUploader that caused flaky test failures
  • Store setTimeout ID and event handler references as instance properties so they can be cleaned up
  • Add destroy() method to BatchUploader that clears timers and removes event listeners
  • Call destroy() before re-init in completeSDKInitialization() and in test beforeEach teardown

Problem

Each SDK re-initialization (and each test run) was leaking:

  1. An orphaned setTimeout from queueUpload() that could fire during a later test
  2. Duplicate visibilitychange, beforeunload, and pagehide listeners that accumulated with every init

This caused non-deterministic test failures depending on timer timing and listener execution order.

@rmi22186 rmi22186 requested a review from a team as a code owner April 17, 2026 15:45
@cursor
Copy link
Copy Markdown

cursor Bot commented Apr 17, 2026

PR Summary

Low Risk
Behavioral changes are limited to cleanup logic (clearing timeouts/removing listeners) and guarding against rescheduling after teardown; main upload/queue logic is unchanged.

Overview
Prevents BatchUploader resource leaks by tracking the upload setTimeout and unload/visibility event handlers as instance state and adding a destroy() method to clear the timer, remove listeners, and stop re-scheduling the interval loop.

SDK re-initialization now explicitly calls uploader.destroy() before replacing _APIClient, and test setup/teardown also destroys the uploader to avoid leaked timers/listeners causing flaky runs.

Reviewed by Cursor Bugbot for commit fee2cc7. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 50239ed. Configure here.

Comment thread src/batchUploader.ts
rmi22186 and others added 2 commits April 17, 2026 11:56
Prevents leaked timers and event listeners from accumulating
across Jest test runs in batchUploader.spec.ts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add a `destroyed` flag to prevent prepareAndUpload from rescheduling
a new timer via triggerUploadInterval if destroy() was called while
an upload was in-flight.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

@rmi22186 rmi22186 merged commit 79ca4f8 into development Apr 17, 2026
30 of 33 checks passed
github-actions Bot pushed a commit that referenced this pull request Apr 23, 2026
# [2.65.0](v2.64.1...v2.65.0) (2026-04-23)

### Bug Fixes

* Clean up BatchUploader timers and event listeners on re-init ([#1251](#1251)) ([79ca4f8](79ca4f8))
* honor config.domain when directURLRouting is enabled ([#1253](#1253)) ([8b64e8e](8b64e8e))

### Features

* set $NoTargeting as User Attribute value ([#1247](#1247)) ([6fa48bf](6fa48bf))
@mparticle-automation
Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 2.65.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants