[pull] develop from RocketChat:develop

.changeset/client-safe-errors-not-exceptions.md

@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+Fixes client-safe errors being reported as exceptions to the channel configured in `Log Exceptions to Channel` when `Log_Level` is set to `2`.

apps/meteor/server/api/lib/logMethodCallError.spec.ts

@@ -0,0 +1,82 @@
+import { expect } from 'chai';
+import { describe, it, before, beforeEach } from 'mocha';
+import proxyquire from 'proxyquire';
+import sinon from 'sinon';
+
+const debugMock = sinon.stub();
+const errorMock = sinon.stub();
+const meteorDebugMock = sinon.stub();
+const settingsGetMock = sinon.stub();
+
+describe('logMethodCallError', () => {
+	let logMethodCallError: (method: string, err: unknown) => void;
+
+	before(() => {
+		({ logMethodCallError } = proxyquire.noCallThru().load('./logMethodCallError', {
+			'meteor/meteor': {
+				Meteor: { _debug: meteorDebugMock },
+			},
+			'../../lib/logger/system': {
+				SystemLogger: { debug: debugMock, error: errorMock },
+			},
+			'../../settings': {
+				settings: { get: settingsGetMock },
+			},
+		}));
+	});
+
+	beforeEach(() => {
+		debugMock.reset();
+		errorMock.reset();
+		meteorDebugMock.reset();
+		settingsGetMock.reset();
+		settingsGetMock.withArgs('Log_Level').returns('2');
+	});
+
+	it('should not report client-safe errors as exceptions', () => {
+		const err = { isClientSafe: true, error: 'error-invalid-user' };
+
+		logMethodCallError('loadHistory', err);
+
+		expect(meteorDebugMock.called).to.be.false;
+		expect(errorMock.called).to.be.false;
+		expect(debugMock.calledOnce).to.be.true;
+		expect(debugMock.calledWithExactly({ msg: 'Expected error while invoking method', err, method: 'loadHistory' })).to.be.true;
+	});
+
+	it('should not report meteor errors as exceptions', () => {
+		const err = { meteorError: { error: 'totp-required' } };
+
+		logMethodCallError('login', err);
+
+		expect(meteorDebugMock.called).to.be.false;
+		expect(errorMock.called).to.be.false;
+		expect(debugMock.calledOnce).to.be.true;
+		expect(debugMock.calledWithExactly({ msg: 'Expected error while invoking method', err, method: 'login' })).to.be.true;
+	});
+
+	it('should report unexpected errors as exceptions when Log_Level is 2', () => {
+		const err = new Error('Match error: Expected string, got number');
+
+		logMethodCallError('loadHistory', err);
+
+		expect(debugMock.called).to.be.false;
+		expect(errorMock.calledOnce).to.be.true;
+		expect(errorMock.calledWithExactly({ msg: 'Exception while invoking method', err, method: 'loadHistory' })).to.be.true;
+		expect(meteorDebugMock.calledOnce).to.be.true;
+		expect(meteorDebugMock.calledWithExactly('Exception while invoking method loadHistory', err)).to.be.true;
+	});
+
+	it('should report unexpected errors without notifying the exceptions channel when Log_Level is not 2', () => {
+		settingsGetMock.withArgs('Log_Level').returns('0');
+
+		const err = new Error('Match error: Expected string, got number');
+
+		logMethodCallError('loadHistory', err);
+
+		expect(debugMock.called).to.be.false;
+		expect(errorMock.calledOnce).to.be.true;
+		expect(errorMock.calledWithExactly({ msg: 'Exception while invoking method', err, method: 'loadHistory' })).to.be.true;
+		expect(meteorDebugMock.called).to.be.false;
+	});
+});

apps/meteor/server/api/lib/logMethodCallError.ts

@@ -0,0 +1,17 @@
+import { Meteor } from 'meteor/meteor';
+
+import { SystemLogger } from '../../lib/logger/system';
+import { settings } from '../../settings';
+
+export function logMethodCallError(method: string, err: unknown): void {
+	if ((err as any)?.isClientSafe || (err as any)?.meteorError) {
+		SystemLogger.debug({ msg: 'Expected error while invoking method', err, method });
+		return;
+	}
+
+	SystemLogger.error({ msg: 'Exception while invoking method', err, method });
+
+	if (settings.get('Log_Level') === '2') {
+		Meteor._debug(`Exception while invoking method ${method}`, err);
+	}
+}

apps/meteor/server/api/v1/misc.ts

@@ -25,7 +25,6 @@ import { Meteor } from 'meteor/meteor';
 
 import { passwordPolicy } from '../../lib/auth/passwordPolicy';
 import { i18n } from '../../lib/i18n';
-import { SystemLogger } from '../../lib/logger/system';
 import { notifyOnSettingChangedById } from '../../lib/notifyListener';
 import { getBaseUserFields } from '../../lib/utils/functions/getBaseUserFields';
 import { isSMTPConfigured } from '../../lib/utils/functions/isSMTPConfigured';
@@ -38,6 +37,7 @@ import { API } from '../api';
 import { getPaginationItems } from '../lib/getPaginationItems';
 import { getUserFromParams } from '../lib/getUserFromParams';
 import { getUserInfo } from '../lib/getUserInfo';
+import { logMethodCallError } from '../lib/logMethodCallError';
 
 /**
  * @openapi
@@ -670,13 +670,7 @@ API.v1.post(
 
 			return API.v1.success(mountResult({ id, result: await Meteor.callAsync(method, ...params) }));
 		} catch (err) {
-			if (!(err as any).isClientSafe && !(err as any).meteorError) {
-				SystemLogger.error({ msg: 'Exception while invoking method', err, method });
-			}
-
-			if (settings.get('Log_Level') === '2') {
-				Meteor._debug(`Exception while invoking method ${method}`, err);
-			}
+			logMethodCallError(method, err);
 
 			return API.v1.failure(mountResult({ id, error: err }));
 		}
@@ -732,12 +726,8 @@ API.v1.post(
 
 			return API.v1.success(mountResult({ id, result: await Meteor.callAsync(method, ...params) }));
 		} catch (err) {
-			if (!(err as any).isClientSafe && !(err as any).meteorError) {
-				SystemLogger.error({ msg: 'Exception while invoking method', err, method });
-			}
-			if (settings.get('Log_Level') === '2') {
-				Meteor._debug(`Exception while invoking method ${method}`, err);
-			}
+			logMethodCallError(method, err);
+
 			return API.v1.failure(mountResult({ id, error: err }));
 		}
 	},