fix: handle shouldAddRateLimitToRoute exception to prevent the server from crashing

[julio-rocketchat]

This issue was likely introduced when removing Restivus and refactoring the API routing: #34999.

Proposed changes (including videos or screenshots)

When running the develop branch with yarn dsv instead of TEST_MODE=TRUE yarn dsv we get the following error:

@rocket.chat/meteor:dsv: W20250205-07:14:37.576(1)? (STDERR) === UnCaughtException ===
@rocket.chat/meteor:dsv: W20250205-07:14:37.576(1)? (STDERR) errorClass [Error]: [You must set "intervalTimeInMS" property in rateLimiter for REST API endpoint]

[..snip]

@rocket.chat/meteor:dsv: => Exited with code: 1
@rocket.chat/meteor:dsv: => Your application is crashing. Waiting for file change.

This error originates from Rocket.Chat/apps/meteor/app/api/server/api.ts, more specifically in the function responsible for refreshing API routes and applying rate-limiting:

	public reloadRoutesToRefreshRateLimiter(): void {
		this._routes.forEach((route) => {
			if (this.shouldAddRateLimitToRoute(route.options)) {
				this.addRateLimiterRuleForRoutes({
					routes: [route.path],
					rateLimiterOptions: route.options.rateLimiterOptions || defaultRateLimiterOptions,
					endpoints: Object.keys(route.endpoints).filter((endpoint) => endpoint !== 'options'),
				});
			}
		});
	}

This function:

1. Checks if rate-limiting should be applied using shouldAddRateLimitToRoute(route.options).
2. Applies the rate-limiter using addRateLimiterRuleForRoutes if the check returns true.

The function addRateLimiterRuleForRoutes enforces that both numRequestsAllowed and intervalTimeInMS must be defined - otherwise, it will throw an error:

protected addRateLimiterRuleForRoutes({
		routes,
		rateLimiterOptions,
		endpoints,
	}: {
		routes: string[];
		rateLimiterOptions: RateLimiterOptions | boolean;
		endpoints: string[];
	}): void {
		if (typeof rateLimiterOptions !== 'object') {
			throw new Meteor.Error('"rateLimiterOptions" must be an object');
		}
		if (!rateLimiterOptions.numRequestsAllowed) {
			throw new Meteor.Error('You must set "numRequestsAllowed" property in rateLimiter for REST API endpoint');
		}
		if (!rateLimiterOptions.intervalTimeInMS) {
			throw new Meteor.Error('You must set "intervalTimeInMS" property in rateLimiter for REST API endpoint');
		}

[...snip]

However, shouldAddRateLimitToRoute does not strictly enforce that numRequestsAllowed and intervalTimeInMS are defined - instead, it only checks whether rateLimiterOptions is an object:

	protected shouldAddRateLimitToRoute(options: { rateLimiterOptions?: RateLimiterOptions | boolean }): boolean {
		const { rateLimiterOptions } = options;
		return (
			(typeof rateLimiterOptions === 'object' || rateLimiterOptions === undefined) &&
			Boolean(this.version) &&
			!process.env.TEST_MODE &&
			Boolean(defaultRateLimiterOptions.numRequestsAllowed && defaultRateLimiterOptions.intervalTimeInMS)
		);
	}

As a result, the following configuration evaluates to true - even if settings.get('API_Enable_Rate_Limiter_Limit_Time_Default') is undefined (which can happen when the server starts for the first time), leading to the uncaught exception:

API.v1.addRoute(
	'users.register',
	{
		authRequired: false,
		rateLimiterOptions: {
			numRequestsAllowed: settings.get('Rate_Limiter_Limit_RegisterUser') ?? 1,
			intervalTimeInMS: settings.get('API_Enable_Rate_Limiter_Limit_Time_Default'),
		},
		validateParams: isUserRegisterParamsPOST,
	},

This PR handles the exception to prevent the server from crashing and adds a fallback value to intervalTimeInMS: settings.get('API_Enable_Rate_Limiter_Limit_Time_Default') in the users.register route.

Issue(s)

SB-763

Steps to test or reproduce

Fetch latest develop branch and start the server with yarn dsv.

Further comments

• Tested on a Macbook Air M2.
• It may only happen with new servers since older servers will likely have the necessary default values initiated.

[dionisio-bot]

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

[changeset-bot]

⚠️ No Changeset found

Latest commit: 7d6b0cf

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.22%. Comparing base (4480b28) to head (7d6b0cf).
Report is 1 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #35117   +/-   ##
========================================
  Coverage    59.22%   59.22%           
========================================
  Files         2824     2824           
  Lines        67980    67980           
  Branches     15117    15117           
========================================
  Hits         40259    40259           
  Misses       24892    24892           
  Partials      2829     2829           

Flag	Coverage Δ
unit	75.17% <ø> (ø)

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

[github-actions]

PR Preview Action v1.6.0
🚀 View preview at https://RocketChat.github.io/Rocket.Chat/pr-preview/pr-35117/
Built to branch gh-pages at 2025-02-05 06:46 UTC. Preview will be ready when the GitHub Pages deployment is complete.

[imunique-ZJ]

Hi, I am curious about why the fallback value is set to 600000 while the default value in API_RATE_LIMIT setting is 60000.

Rocket.Chat/apps/meteor/server/settings/rate.ts

Lines 67 to 70 in 1198e51

	await this.add('API_Enable_Rate_Limiter_Limit_Time_Default', 60000, {
	type: 'int',
	enableQuery: { _id: 'API_Enable_Rate_Limiter', value: true },
	});

[julio-rocketchat]

Hi, I am curious about why the fallback value is set to 600000 while the default value in API_RATE_LIMIT setting is 60000.

Rocket.Chat/apps/meteor/server/settings/rate.ts

Lines 67 to 70 in 1198e51

	await this.add('API_Enable_Rate_Limiter_Limit_Time_Default', 60000, {
	type: 'int',
	enableQuery: { _id: 'API_Enable_Rate_Limiter', value: true },
	});

It was solely a typo. Good catch, @imunique-ZJ. Thanks for noticing it and we've opened a PR to fix the typo: #35180. It's pending approval and we will merge it before the next release. Much appreciated