From e10bd504b9e4576d6f862393caa277e695d249ed Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Thu, 20 Aug 2026 10:20:54 +0000 Subject: [PATCH] fix: client-safe errors reported as exceptions to the exceptions channel (#41795) Signed-off-by: Abhinav Kumar --- .../client-safe-errors-not-exceptions.md | 5 ++ .../server/api/lib/logMethodCallError.spec.ts | 82 +++++++++++++++++++ .../server/api/lib/logMethodCallError.ts | 17 ++++ apps/meteor/server/api/v1/misc.ts | 18 +--- 4 files changed, 108 insertions(+), 14 deletions(-) create mode 100644 .changeset/client-safe-errors-not-exceptions.md create mode 100644 apps/meteor/server/api/lib/logMethodCallError.spec.ts create mode 100644 apps/meteor/server/api/lib/logMethodCallError.ts diff --git a/.changeset/client-safe-errors-not-exceptions.md b/.changeset/client-safe-errors-not-exceptions.md new file mode 100644 index 0000000000000..fe2e754edfe7c --- /dev/null +++ b/.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`. diff --git a/apps/meteor/server/api/lib/logMethodCallError.spec.ts b/apps/meteor/server/api/lib/logMethodCallError.spec.ts new file mode 100644 index 0000000000000..8ebc140186524 --- /dev/null +++ b/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; + }); +}); diff --git a/apps/meteor/server/api/lib/logMethodCallError.ts b/apps/meteor/server/api/lib/logMethodCallError.ts new file mode 100644 index 0000000000000..bdab9458b4724 --- /dev/null +++ b/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); + } +} diff --git a/apps/meteor/server/api/v1/misc.ts b/apps/meteor/server/api/v1/misc.ts index 8ae27a6708328..a8fceb93eec14 100644 --- a/apps/meteor/server/api/v1/misc.ts +++ b/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 })); } },