From e72a8d988678738b239e22a32b4826930852959f Mon Sep 17 00:00:00 2001 From: Sebastian Kreft Date: Wed, 4 Sep 2024 19:05:44 -0400 Subject: [PATCH] fix: correctly truncate seenEvents (#282) The logic to truncate a set was incorrect and could produce and almost inifinite loop as it was decreasing the counter rather than increasing it. We extracted the logic to its own module, where it can be tested. --- CHANGELOG.md | 6 ++++++ src/detector.ts | 9 ++------- src/set.test.ts | 17 +++++++++++++++++ src/set.ts | 10 ++++++++++ 4 files changed, 35 insertions(+), 7 deletions(-) create mode 100644 src/set.test.ts create mode 100644 src/set.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 5614c4d..be6ace2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ The format is loosely based on [Keep a Changelog](https://keepachangelog.com/en/ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). We follow the format used by [Open Telemetry](https://github.com/open-telemetry/opentelemetry-python/blob/main/CHANGELOG.md). +## Unreleased + +### Fixed +- Fix truncation of `seenEvents` + ([#282](https://github.com/Topsort/analytics.js/pull/282)) + ## Version 2.3.1 (2024-04-11) Patch release to fix tags diff --git a/src/detector.ts b/src/detector.ts index bb319d8..bf3524e 100644 --- a/src/detector.ts +++ b/src/detector.ts @@ -1,6 +1,7 @@ import { type Config, Entity, TopsortEvent, reportEvent } from "@topsort/sdk"; import { version } from "../package.json"; import { ProcessorResult, Queue } from "./queue"; +import { truncateSet } from "./set"; import { BidStore } from "./store"; const MAX_EVENTS_SIZE = 2500; @@ -181,13 +182,7 @@ function logEvent(info: ProductEvent, node: Node) { return; } seenEvents.add(id); - if (seenEvents.size > MAX_EVENTS_SIZE) { - const iterator = seenEvents.values(); - for (let i = 0; i < seenEvents.size - MAX_EVENTS_SIZE; --i) { - iterator.next(); - } - seenEvents = new Set(iterator); - } + seenEvents = truncateSet(seenEvents, MAX_EVENTS_SIZE); queue.append(info); // Raise a custom event, so that clients can trigger their own logic. diff --git a/src/set.test.ts b/src/set.test.ts new file mode 100644 index 0000000..350ab32 --- /dev/null +++ b/src/set.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, test } from "vitest"; +import { truncateSet } from "./set"; + +describe("truncateSet", () => { + test("original set below maxSize", () => { + const s = new Set([1, 2, 3]); + expect(truncateSet(s, 4)).toBe(s); + }); + + test("resulting set keeps last inserted items", () => { + const s = new Set([1, 2, 3]); + s.add(4); + expect(truncateSet(s, 3)).toEqual(new Set([2, 3, 4])); + expect(truncateSet(s, 2)).toEqual(new Set([3, 4])); + expect(truncateSet(s, 1)).toEqual(new Set([4])); + }); +}); diff --git a/src/set.ts b/src/set.ts new file mode 100644 index 0000000..6ab3742 --- /dev/null +++ b/src/set.ts @@ -0,0 +1,10 @@ +export function truncateSet(set: Set, maxSize: number): Set { + if (set.size <= maxSize) { + return set; + } + const iterator = set.values(); + for (let i = 0; i < set.size - maxSize; ++i) { + iterator.next(); + } + return new Set(iterator); +}