Skip to content

Commit

Permalink
fix: correctly truncate seenEvents (#282)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sk- authored Sep 4, 2024
1 parent 5d62bf5 commit e72a8d9
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 7 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 2 additions & 7 deletions src/detector.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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.
Expand Down
17 changes: 17 additions & 0 deletions src/set.test.ts
Original file line number Diff line number Diff line change
@@ -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]));
});
});
10 changes: 10 additions & 0 deletions src/set.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export function truncateSet<T>(set: Set<T>, maxSize: number): Set<T> {
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);
}

0 comments on commit e72a8d9

Please sign in to comment.