chore: throttle exceptions counter increment #29719

sampaiodiego · 2023-07-04T17:02:33Z

Proposed changes (including videos or screenshots)

Throttle down uncaught exceptions counter to every 10 seconds. This is to avoid too many operations into the settings collection.

Issue(s)

Steps to test or reproduce

Further comments

changeset-bot · 2023-07-04T17:02:37Z

⚠️ No Changeset found

Latest commit: 5661935

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

ggazzo · 2023-07-04T17:42:20Z

packages/tools/src/counter.ts

+import throttle from 'lodash.throttle';
+
+export function throttledCounter(fn: (counter: number) => unknown, wait: number) {
+	let counter = 0;
+
+	const throttledFn = throttle(
+		() => {
+			fn(counter);
+
+			counter = 0;
+		},
+		wait,
+		{ leading: false },
+	);
+
+	return () => {
+		counter++;
+		throttledFn();
+	};
+}


I dont think this should be placed here, its not quite useful outside that scope

not sure about the lodash necessity as well

codecov · 2023-07-04T18:16:20Z

Codecov Report

Merging #29719 (5661935) into develop (c10137e) will increase coverage by 15.25%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           develop   #29719       +/-   ##
============================================
+ Coverage    31.33%   46.58%   +15.25%     
============================================
  Files          570      693      +123     
  Lines        10839    12946     +2107     
  Branches      1993     2242      +249     
============================================
+ Hits          3396     6031     +2635     
+ Misses        7229     6589      -640     
- Partials       214      326      +112

Flag	Coverage Δ
e2e	`46.55% <ø> (+15.22%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

KevLehman · 2023-07-04T18:41:25Z

apps/meteor/app/error-handler/server/lib/RocketChat.ErrorHandler.ts


 import { settings } from '../../../settings/server';
 import { sendMessage } from '../../../lib/server';

+const incException = throttledCounter((counter) => {


wdyt of ignoring this setting on the watch.settings watcher?

the main issue is that we'd still receive it from mongo.. so we'd need to remove it on oplog/change stream query, which is not good 😬

ggazzo · 2023-07-04T19:09:28Z

packages/tools/src/counter.ts

+import throttle from 'lodash.throttle';
+
+export function throttledCounter(fn: (counter: number) => unknown, wait: number) {
+	let counter = 0;
+
+	const throttledFn = throttle(
+		() => {
+			fn(counter);
+
+			counter = 0;
+		},
+		wait,
+		{ leading: false },
+	);
+
+	return () => {
+		counter++;
+		throttledFn();
+	};
+}


Suggested change

import throttle from 'lodash.throttle';

export function throttledCounter(fn: (counter: number) => unknown, wait: number) {

let counter = 0;

const throttledFn = throttle(

() => {

fn(counter);

counter = 0;

},

wait,

{ leading: false },

);

return () => {

counter++;

throttledFn();

};

}

export const throttle = <R, A extends any[]>(

fn: (...args: A) => R,

delay: number

) => {

let wait = false;

let timeout: undefined | number;

return Object.assign(

(...args: A) => {

if (wait) return undefined;

const val = fn(...args);

wait = true;

timeout = window.setTimeout(() => {

wait = false;

}, delay);

return val;

}, {

stop: () => {

clearTimeout(timeout);

},

});

};

ggazzo · 2023-07-04T19:09:45Z

apps/meteor/app/error-handler/server/lib/RocketChat.ErrorHandler.ts

@@ -1,9 +1,14 @@
 import { Meteor } from 'meteor/meteor';
 import { Settings, Users, Rooms } from '@rocket.chat/models';
+import { throttledCounter } from '@rocket.chat/tools';


Suggested change

import { throttledCounter } from '@rocket.chat/tools';

import { throttle } from '@rocket.chat/tools';

ggazzo · 2023-07-04T19:09:59Z

apps/meteor/app/error-handler/server/lib/RocketChat.ErrorHandler.ts

+const incException = throttledCounter((counter) => {
+	Settings.incrementValueById('Uncaught_Exceptions_Count', counter).catch(console.error);
+}, 10000);
+


Suggested change

const incException = throttledCounter((counter) => {

Settings.incrementValueById('Uncaught_Exceptions_Count', counter).catch(console.error);

}, 10000);

const incException = (() => {

let counter = 0;

const commit = throttle((increment: number) => {

console.log(counter);

counter = 0;

}, 10000);

return () => {

counter++;

commit(counter);

};

})()

…/reportUser * 'develop' of github.com:RocketChat/Rocket.Chat: (28 commits) feat: Add missing variants to UIKit button (#29654) refactor: Remove Accountbox usage (#29786) chore: stop importing action manager as global (#29766) chore: create FeaturePreview Component (#29759) regression: add missing translations on MenuV2 replace (#29777) test: create mock package (#29765) fix: Prevent app's bridges from overriding lastMsg prop on rooms col (#29756) regression: option to start video conferences disappearing for regular users (#29763) Release 6.2.9 Revert "Release 6.2.9" fix(meteor): `room-opened` event not dispatching when navigating cached rooms (#29718) chore: device management improve readability and performance (#29712) chore: throttle exceptions counter increment (#29719) chore: Improve performance of getActiveLocalUserCount (#29681) Release 6.2.9 ci: update EE license (#29684) ci: Add missing `context` property on compose file (#29676) chore: add change stream option to get full doc (#29627) fix(meteor): Message composer keeps recording even though permission was denied (#29526) fix(meteor): Video Record button disabled on iOS browsers (#29649) ...

chore: throttle exceptions counter increment

837a210

sampaiodiego added the stat: QA skipped label Jul 4, 2023

sampaiodiego added this to the 6.2.10 milestone Jul 4, 2023

sampaiodiego requested review from geekgonecrazy and rodrigok July 4, 2023 17:02

rodrigok previously approved these changes Jul 4, 2023

View reviewed changes

chore: throttle down handleMaxSeatsBanners

1285254

ggazzo reviewed Jul 4, 2023

View reviewed changes

add dep to meteor

32a25e3

sampaiodiego dismissed rodrigok’s stale review via 32a25e3 July 4, 2023 17:43

KevLehman reviewed Jul 4, 2023

View reviewed changes

ggazzo reviewed Jul 4, 2023

View reviewed changes

move function out from package

5661935

sampaiodiego force-pushed the throttle-exceptions-counter branch from 03be8c3 to 5661935 Compare July 4, 2023 20:12

sampaiodiego requested a review from rodrigok July 4, 2023 20:17

ggazzo approved these changes Jul 4, 2023

View reviewed changes

sampaiodiego merged commit 706561a into develop Jul 4, 2023

sampaiodiego deleted the throttle-exceptions-counter branch July 4, 2023 20:54

sampaiodiego added a commit that referenced this pull request Jul 5, 2023

chore: throttle exceptions counter increment (#29719)

154bafc

sampaiodiego modified the milestones: 6.2.10, 6.2.9 Jul 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: throttle exceptions counter increment #29719

chore: throttle exceptions counter increment #29719

sampaiodiego commented Jul 4, 2023

changeset-bot bot commented Jul 4, 2023 •

edited

Loading

ggazzo Jul 4, 2023

codecov bot commented Jul 4, 2023 •

edited

Loading

KevLehman Jul 4, 2023

sampaiodiego Jul 4, 2023

ggazzo Jul 4, 2023

ggazzo Jul 4, 2023

ggazzo Jul 4, 2023

-import throttle from 'lodash.throttle';
-export function throttledCounter(fn: (counter: number) => unknown, wait: number) {
-	let counter = 0;
-	const throttledFn = throttle(
-		() => {
-			fn(counter);
-			counter = 0;
-		},
-		wait,
-		{ leading: false },
-	);
-	return () => {
-		counter++;
-		throttledFn();
-	};
-}
+export const throttle = <R, A extends any[]>(
+    fn: (...args: A) => R,
+    delay: number
+) => {
+    let wait = false;
+    let timeout: undefined | number;
+    return Object.assign(
+        (...args: A) => {
+            if (wait) return undefined;
+            const val = fn(...args);
+            wait = true;
+            timeout = window.setTimeout(() => {
+                wait = false;
+            }, delay);
+            return val;
+        }, {
+        stop: () => {
+            clearTimeout(timeout);
+        },
+    });
+};

	import { throttledCounter } from '@rocket.chat/tools';
	import { throttle } from '@rocket.chat/tools';

chore: throttle exceptions counter increment #29719

chore: throttle exceptions counter increment #29719

Conversation

sampaiodiego commented Jul 4, 2023

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

changeset-bot bot commented Jul 4, 2023 • edited Loading

⚠️ No Changeset found

ggazzo Jul 4, 2023

Choose a reason for hiding this comment

codecov bot commented Jul 4, 2023 • edited Loading

Codecov Report

KevLehman Jul 4, 2023

Choose a reason for hiding this comment

sampaiodiego Jul 4, 2023

Choose a reason for hiding this comment

ggazzo Jul 4, 2023

Choose a reason for hiding this comment

ggazzo Jul 4, 2023

Choose a reason for hiding this comment

ggazzo Jul 4, 2023

Choose a reason for hiding this comment

changeset-bot bot commented Jul 4, 2023 •

edited

Loading

codecov bot commented Jul 4, 2023 •

edited

Loading