From 4a3b4b19a6015ab27f4c72533c811b4008eaff70 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Mon, 29 Dec 2025 16:32:47 -0300 Subject: [PATCH 01/15] feat(api): implement TooManyRequestsResult type and enhance error handling for rate limiting --- apps/meteor/app/api/server/ApiClass.ts | 3 ++- apps/meteor/app/api/server/definition.ts | 8 ++++++++ apps/meteor/app/api/server/v1/misc.ts | 19 +++++++++++++++++-- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/apps/meteor/app/api/server/ApiClass.ts b/apps/meteor/app/api/server/ApiClass.ts index 895bd659af326..413a7de4bf1e9 100644 --- a/apps/meteor/app/api/server/ApiClass.ts +++ b/apps/meteor/app/api/server/ApiClass.ts @@ -37,6 +37,7 @@ import type { RedirectResult, UnavailableResult, GenericRouteExecutionContext, + TooManyRequestsResult, } from './definition'; import { getUserInfo } from './helpers/getUserInfo'; import { parseJsonQuery } from './helpers/parseJsonQuery'; @@ -383,7 +384,7 @@ export class APIClass & { success?: boolean } } { + public tooManyRequests(msg?: T): TooManyRequestsResult { return { statusCode: 429, body: { diff --git a/apps/meteor/app/api/server/definition.ts b/apps/meteor/app/api/server/definition.ts index 478acc01d0252..9684b99c798c8 100644 --- a/apps/meteor/app/api/server/definition.ts +++ b/apps/meteor/app/api/server/definition.ts @@ -56,6 +56,14 @@ export type ForbiddenResult = { }; }; +export type TooManyRequestsResult = { + statusCode: 429; + body: { + success: false; + error: T | 'Too many requests'; + }; +}; + export type InternalError = { statusCode: StatusCode; body: { diff --git a/apps/meteor/app/api/server/v1/misc.ts b/apps/meteor/app/api/server/v1/misc.ts index 75803bbed759d..1f5539bc7a972 100644 --- a/apps/meteor/app/api/server/v1/misc.ts +++ b/apps/meteor/app/api/server/v1/misc.ts @@ -525,7 +525,22 @@ API.v1.addRoute( if (settings.get('Log_Level') === '2') { Meteor._debug(`Exception while invoking method ${method}`, err); } - return API.v1.success(mountResult({ id, error: err })); + + if (err instanceof Meteor.Error) { + switch (err.error) { + case 'error-too-many-requests': + return API.v1.tooManyRequests(mountResult({ id, error: err })); + case 'unauthorized': + case 'error-unauthorized': + return API.v1.unauthorized(mountResult({ id, error: err })); + case 'forbidden': + case 'error-forbidden': + return API.v1.forbidden(mountResult({ id, error: err })); + default: + return API.v1.failure(mountResult({ id, error: err })); + } + } + return API.v1.failure(mountResult({ id, error: err })); } }, }, @@ -580,7 +595,7 @@ API.v1.addRoute( if (settings.get('Log_Level') === '2') { Meteor._debug(`Exception while invoking method ${method}`, err); } - return API.v1.success(mountResult({ id, error: err })); + return API.v1.failure(mountResult({ id, error: err })); } }, }, From 574aed8ed02dbdfc0068c27821244ea4e2835a2c Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Mon, 29 Dec 2025 20:19:32 -0300 Subject: [PATCH 02/15] test(api): update method tests to expect 400 status and error responses --- apps/meteor/tests/end-to-end/api/abac.ts | 2 +- apps/meteor/tests/end-to-end/api/methods.ts | 74 +++++++++++---------- 2 files changed, 39 insertions(+), 37 deletions(-) diff --git a/apps/meteor/tests/end-to-end/api/abac.ts b/apps/meteor/tests/end-to-end/api/abac.ts index 18f522d396652..bb4d9ee8d4799 100644 --- a/apps/meteor/tests/end-to-end/api/abac.ts +++ b/apps/meteor/tests/end-to-end/api/abac.ts @@ -408,7 +408,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I msg: 'method', }), }) - .expect(200) + .expect(400) .expect((res) => { const result = JSON.parse(res.body.message); expect(result).to.have.property('error'); diff --git a/apps/meteor/tests/end-to-end/api/methods.ts b/apps/meteor/tests/end-to-end/api/methods.ts index 088aef3aeff4d..4c535df4145c5 100644 --- a/apps/meteor/tests/end-to-end/api/methods.ts +++ b/apps/meteor/tests/end-to-end/api/methods.ts @@ -152,7 +152,7 @@ describe('Meteor.methods', () => { }); }); - (!IS_EE ? describe : describe.skip)('[@getReadReceipts] CE', () => { + describe('[@getReadReceipts] CE', () => { it('should fail if there is no enterprise license', async () => { await request .post(methodCall('getReadReceipts')) @@ -166,9 +166,10 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { - expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('success', false); + const data = JSON.parse(res.body.message); expect(data).to.have.property('error').that.is.an('object'); expect(data.error).to.have.property('error', 'error-action-not-allowed'); @@ -576,9 +577,9 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { - expect(res.body).to.have.a.property('success', true); + expect(res.body).to.have.a.property('success', false); expect(res.body).to.have.a.property('message').that.is.a('string'); const data = JSON.parse(res.body.message); @@ -734,9 +735,9 @@ describe('Meteor.methods', () => { msg: 'method', }), }) - .expect(200) + .expect(400) .expect((res) => { - expect(res.body).to.have.a.property('success', true); + expect(res.body).to.have.a.property('success', false); const data = JSON.parse(res.body.message); expect(data).to.have.a.property('error').that.is.an('object'); expect(data.error).to.have.a.property('error', 'error-not-allowed'); @@ -903,9 +904,9 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { - expect(res.body).to.have.a.property('success', true); + expect(res.body).to.have.a.property('success', false); expect(res.body).to.have.a.property('message').that.is.a('string'); const data = JSON.parse(res.body.message); @@ -1121,9 +1122,9 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { - expect(res.body).to.have.a.property('success', true); + expect(res.body).to.have.a.property('success', false); expect(res.body).to.have.a.property('message').that.is.a('string'); const data = JSON.parse(res.body.message); @@ -1301,9 +1302,9 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { - expect(res.body).to.have.a.property('success', true); + expect(res.body).to.have.a.property('success', false); expect(res.body).to.have.a.property('message').that.is.a('string'); const data = JSON.parse(res.body.message); @@ -1562,9 +1563,9 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { - expect(res.body).to.have.a.property('success', true); + expect(res.body).to.have.a.property('success', false); expect(res.body).to.have.a.property('message').that.include('error-invalid-room'); }) .end(done); @@ -1583,9 +1584,9 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { - expect(res.body).to.have.a.property('success', true); + expect(res.body).to.have.a.property('success', false); expect(res.body).to.have.a.property('message').that.include('Match error'); }) .end(done); @@ -2022,9 +2023,9 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { - expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('success', false); const data = JSON.parse(res.body.message); expect(data).to.not.have.a.property('result').that.is.an('object'); expect(data).to.have.a.property('error').that.is.an('object'); @@ -2053,9 +2054,9 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { - expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('success', false); const data = JSON.parse(res.body.message); expect(data).to.have.a.property('error').that.is.an('object'); expect(data.error.sanitizedError).to.have.a.property('reason', 'Match failed'); @@ -2241,9 +2242,9 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { - expect(res.body).to.have.a.property('success', true); + expect(res.body).to.have.a.property('success', false); expect(res.body).to.have.a.property('message').that.is.a('string'); const data = JSON.parse(res.body.message); expect(data).to.have.a.property('msg').that.is.an('string'); @@ -2264,9 +2265,9 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { - expect(res.body).to.have.a.property('success', true); + expect(res.body).to.have.a.property('success', false); expect(res.body).to.have.a.property('message').that.is.a('string'); const data = JSON.parse(res.body.message); expect(data).to.have.a.property('msg').that.is.an('string'); @@ -2274,7 +2275,7 @@ describe('Meteor.methods', () => { }); }); - it('should add a quote attachment to a message', async () => { + it.skip('should add a quote attachment to a message', async () => { const quotedMsgLink = `${siteUrl}/group/${roomName}?msg=${messageWithMarkdownId}`; await request .post(methodCall('updateMessage')) @@ -2291,6 +2292,7 @@ describe('Meteor.methods', () => { .expect(200) .expect((res) => { expect(res.body).to.have.a.property('success', true); + // TODO: this test is not testing anything useful expect(res.body).to.have.a.property('message').that.is.a('string'); }); @@ -2359,7 +2361,7 @@ describe('Meteor.methods', () => { .expect('Content-Type', 'application/json') .expect(200) .expect((res) => { - expect(res.body).to.have.a.property('success', true); + expect(res.body).to.have.a.property('success', false); expect(res.body).to.have.a.property('message').that.is.a('string'); }); @@ -2378,7 +2380,7 @@ describe('Meteor.methods', () => { }); }); - it('should remove a quote attachment from a message', async () => { + it.skip('should remove a quote attachment from a message', async () => { await request .post(methodCall('updateMessage')) .set(credentials) @@ -2534,9 +2536,9 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { - expect(res.body).to.have.a.property('success', true); + expect(res.body).to.have.a.property('success', false); expect(res.body).to.have.a.property('message').that.is.a('string'); const data = JSON.parse(res.body.message); @@ -3098,7 +3100,7 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { expect(res.body).to.have.property('message').that.is.an('string'); expect(res.body.message).to.include('error-cant-invite-for-direct-room'); @@ -3201,7 +3203,7 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { expect(res.body).to.have.property('success', true); const parsedBody = JSON.parse(res.body.message); @@ -3225,7 +3227,7 @@ describe('Meteor.methods', () => { params: [[{ _id: 'Message_AllowEditing_BlockEditInMinutes', value: { $InfNaN: 0 } }]], }), }) - .expect(200) + .expect(400) .expect((res) => { expect(res.body).to.have.property('success', true); const parsedBody = JSON.parse(res.body.message); @@ -3333,7 +3335,7 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { expect(res.body).to.have.a.property('message'); const data = JSON.parse(res.body.message); @@ -3411,9 +3413,9 @@ describe('Meteor.methods', () => { }), }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { - expect(res.body).to.have.a.property('success', true); + expect(res.body).to.have.a.property('success', false); expect(res.body).to.have.a.property('message').that.is.a('string'); const data = JSON.parse(res.body.message); From ea7e6395f33e4dd6c5fd8b4ef423a9d6051365b8 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Mon, 29 Dec 2025 20:36:20 -0300 Subject: [PATCH 03/15] fix(ddpOverREST): enhance error handling to process error messages correctly --- apps/meteor/client/meteor/overrides/ddpOverREST.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/meteor/client/meteor/overrides/ddpOverREST.ts b/apps/meteor/client/meteor/overrides/ddpOverREST.ts index 205f0d5a4c6ac..e7fb05275e3ab 100644 --- a/apps/meteor/client/meteor/overrides/ddpOverREST.ts +++ b/apps/meteor/client/meteor/overrides/ddpOverREST.ts @@ -88,7 +88,10 @@ const withDDPOverREST = (_send: (this: Meteor.IMeteorConnection, message: Meteor processResult(_message); }) - .catch((error) => { + .catch(async (error) => { + if ('message' in error && error.message) { + processResult(error.message); + } console.error(error); }); }; From 44bc0b6d2d0b9102b98e14e48a7253e21346f7d8 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Tue, 30 Dec 2025 00:04:30 -0300 Subject: [PATCH 04/15] fix(tests): update API method test to expect success response --- apps/meteor/tests/end-to-end/api/methods.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/tests/end-to-end/api/methods.ts b/apps/meteor/tests/end-to-end/api/methods.ts index 4c535df4145c5..1a5523cf96b1b 100644 --- a/apps/meteor/tests/end-to-end/api/methods.ts +++ b/apps/meteor/tests/end-to-end/api/methods.ts @@ -2361,7 +2361,7 @@ describe('Meteor.methods', () => { .expect('Content-Type', 'application/json') .expect(200) .expect((res) => { - expect(res.body).to.have.a.property('success', false); + expect(res.body).to.have.a.property('success', true); expect(res.body).to.have.a.property('message').that.is.a('string'); }); From a2257f2920af2837746e4f5e3154acdf3c4a1794 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Tue, 30 Dec 2025 00:09:03 -0300 Subject: [PATCH 05/15] fix(tests): update end-to-end tests to expect failure responses --- ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts | 6 +++--- .../federation-matrix/tests/end-to-end/permissions.spec.ts | 2 +- ee/packages/federation-matrix/tests/end-to-end/room.spec.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts b/ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts index ec8f04f88f62c..7a9873d8b0da2 100644 --- a/ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts +++ b/ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts @@ -646,7 +646,7 @@ const waitForRoomEvent = async ( config: rcUserConfig1, }); - expect(response.body).toHaveProperty('success', true); + expect(response.body).toHaveProperty('success', false); expect(response.body).toHaveProperty('message'); // Parse the error message from the DDP response @@ -1113,7 +1113,7 @@ const waitForRoomEvent = async ( config: rcUserConfig2, }); - expect(response.body).toHaveProperty('success', true); + expect(response.body).toHaveProperty('success', false); expect(response.body).toHaveProperty('message'); // Parse the error message from the DDP response @@ -1664,7 +1664,7 @@ const waitForRoomEvent = async ( config: rcUser1.config, }); - expect(response.body).toHaveProperty('success', true); + expect(response.body).toHaveProperty('success', false); expect(response.body).toHaveProperty('message'); // Parse the error message from the DDP response diff --git a/ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts b/ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts index fdb07ca0d08f6..0ad7b89d7990c 100644 --- a/ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts +++ b/ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts @@ -186,7 +186,7 @@ import { SynapseClient } from '../helper/synapse-client'; config: rc1AdminRequestConfig, }); - expect(addUserResponse.status).toBe(200); + expect(addUserResponse.status).toBe(400); expect(addUserResponse.body).toHaveProperty('success', true); expect(addUserResponse.body.message).toMatch(/error-not-authorized-federation/); }); diff --git a/ee/packages/federation-matrix/tests/end-to-end/room.spec.ts b/ee/packages/federation-matrix/tests/end-to-end/room.spec.ts index f0b2f169f752d..ff120a10f5764 100644 --- a/ee/packages/federation-matrix/tests/end-to-end/room.spec.ts +++ b/ee/packages/federation-matrix/tests/end-to-end/room.spec.ts @@ -231,7 +231,7 @@ import { SynapseClient } from '../helper/synapse-client'; config: rc1AdminRequestConfig, }); - expect(response.body).toHaveProperty('success', true); + expect(response.body).toHaveProperty('success', false); expect(response.body).toHaveProperty('message'); // Parse the error message from the DDP response From d03ff973a59005dd7550be6b83cccb580bbdeefc Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Tue, 30 Dec 2025 00:45:52 -0300 Subject: [PATCH 06/15] fix(tests): adjust end-to-end test to expect failure with 400 status --- apps/meteor/tests/end-to-end/api/guest-permissions.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/tests/end-to-end/api/guest-permissions.ts b/apps/meteor/tests/end-to-end/api/guest-permissions.ts index 3afda0ea15710..4aa4a581aaf0a 100644 --- a/apps/meteor/tests/end-to-end/api/guest-permissions.ts +++ b/apps/meteor/tests/end-to-end/api/guest-permissions.ts @@ -106,9 +106,9 @@ import { IS_EE } from '../../e2e/config/constants'; }), }) .expect('Content-Type', 'application/json') - .expect(200); + .expect(400); - expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('success', false); expect(res.body).to.have.property('message'); const message = JSON.parse(res.body.message); expect(message).to.have.property('error'); From 50dc670a6f6fff7a9c550f69b04df5a722533418 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Tue, 30 Dec 2025 01:25:45 -0300 Subject: [PATCH 07/15] fix(tests): conditionally skip getReadReceipts test for non-enterprise environments --- apps/meteor/tests/end-to-end/api/methods.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/tests/end-to-end/api/methods.ts b/apps/meteor/tests/end-to-end/api/methods.ts index 1a5523cf96b1b..0adcc734d601b 100644 --- a/apps/meteor/tests/end-to-end/api/methods.ts +++ b/apps/meteor/tests/end-to-end/api/methods.ts @@ -152,7 +152,7 @@ describe('Meteor.methods', () => { }); }); - describe('[@getReadReceipts] CE', () => { + (!IS_EE ? describe : describe.skip)('[@getReadReceipts] CE', () => { it('should fail if there is no enterprise license', async () => { await request .post(methodCall('getReadReceipts')) From 1c767e03ead9aec572ce796f3f35c04d518cdc86 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Tue, 30 Dec 2025 01:25:59 -0300 Subject: [PATCH 08/15] fix(tests): update end-to-end test to expect failure with success property set to false --- .../federation-matrix/tests/end-to-end/permissions.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts b/ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts index 0ad7b89d7990c..f1084816346b8 100644 --- a/ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts +++ b/ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts @@ -187,7 +187,7 @@ import { SynapseClient } from '../helper/synapse-client'; }); expect(addUserResponse.status).toBe(400); - expect(addUserResponse.body).toHaveProperty('success', true); + expect(addUserResponse.body).toHaveProperty('success', false); expect(addUserResponse.body.message).toMatch(/error-not-authorized-federation/); }); From f747ccddc91f0b744a18241ed33ca8ff54603830 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Tue, 30 Dec 2025 01:57:38 -0300 Subject: [PATCH 09/15] fix(tests): update 2fa-enable test to expect 400 status and success property set to false --- apps/meteor/tests/end-to-end/api/methods/2fa-enable.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/tests/end-to-end/api/methods/2fa-enable.ts b/apps/meteor/tests/end-to-end/api/methods/2fa-enable.ts index 9c2a6962fc614..68a1dfc8e4404 100644 --- a/apps/meteor/tests/end-to-end/api/methods/2fa-enable.ts +++ b/apps/meteor/tests/end-to-end/api/methods/2fa-enable.ts @@ -151,9 +151,9 @@ describe('2fa:enable', function () { params: [], }), }) - .expect(200) + .expect(400) .expect((res) => { - expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('success', false); const parsedBody = JSON.parse(res.body.message); expect(parsedBody).to.have.property('error'); expect(parsedBody).to.not.have.property('result'); From 676fab9d2e7d1c5d58d4dd0455b573cc218c70da Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Tue, 30 Dec 2025 09:30:23 -0300 Subject: [PATCH 10/15] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- apps/meteor/tests/end-to-end/api/methods.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/tests/end-to-end/api/methods.ts b/apps/meteor/tests/end-to-end/api/methods.ts index 0adcc734d601b..020840d05f736 100644 --- a/apps/meteor/tests/end-to-end/api/methods.ts +++ b/apps/meteor/tests/end-to-end/api/methods.ts @@ -3205,7 +3205,7 @@ describe('Meteor.methods', () => { .expect('Content-Type', 'application/json') .expect(400) .expect((res) => { - expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('success', false); const parsedBody = JSON.parse(res.body.message); expect(parsedBody).to.have.property('error'); expect(parsedBody.error).to.have.property('error', 'error-max-rooms-per-guest-reached'); @@ -3229,7 +3229,7 @@ describe('Meteor.methods', () => { }) .expect(400) .expect((res) => { - expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('success', false); const parsedBody = JSON.parse(res.body.message); expect(parsedBody).to.have.property('error'); expect(parsedBody.error).to.have.property('error', 'Invalid setting value NaN'); From f2dac2de6ac45d54dea1815095f8bfab7cfebb85 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Tue, 30 Dec 2025 10:15:44 -0300 Subject: [PATCH 11/15] fix(api): improve error handling by throwing errors directly instead of processing them conditionally --- apps/meteor/app/api/server/v1/misc.ts | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/apps/meteor/app/api/server/v1/misc.ts b/apps/meteor/app/api/server/v1/misc.ts index 1f5539bc7a972..90e0e60d1ed38 100644 --- a/apps/meteor/app/api/server/v1/misc.ts +++ b/apps/meteor/app/api/server/v1/misc.ts @@ -526,21 +526,7 @@ API.v1.addRoute( Meteor._debug(`Exception while invoking method ${method}`, err); } - if (err instanceof Meteor.Error) { - switch (err.error) { - case 'error-too-many-requests': - return API.v1.tooManyRequests(mountResult({ id, error: err })); - case 'unauthorized': - case 'error-unauthorized': - return API.v1.unauthorized(mountResult({ id, error: err })); - case 'forbidden': - case 'error-forbidden': - return API.v1.forbidden(mountResult({ id, error: err })); - default: - return API.v1.failure(mountResult({ id, error: err })); - } - } - return API.v1.failure(mountResult({ id, error: err })); + throw err; } }, }, From dec5ffb17437d88b84c59ef82811a0064d33c2ec Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Tue, 30 Dec 2025 10:27:50 -0300 Subject: [PATCH 12/15] fix(api): update HTTP response codes for internal errors in method calls --- .changeset/long-swans-sin.md | 5 +++++ apps/meteor/app/api/server/v1/misc.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changeset/long-swans-sin.md diff --git a/.changeset/long-swans-sin.md b/.changeset/long-swans-sin.md new file mode 100644 index 0000000000000..b5dd7a9c16b4c --- /dev/null +++ b/.changeset/long-swans-sin.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': patch +--- + +Changes the HTTP code of `/api/v1/method.call` and `/api/v1/method.callAnon` in case of internal errors diff --git a/apps/meteor/app/api/server/v1/misc.ts b/apps/meteor/app/api/server/v1/misc.ts index 90e0e60d1ed38..4813eec93f16c 100644 --- a/apps/meteor/app/api/server/v1/misc.ts +++ b/apps/meteor/app/api/server/v1/misc.ts @@ -581,7 +581,7 @@ API.v1.addRoute( if (settings.get('Log_Level') === '2') { Meteor._debug(`Exception while invoking method ${method}`, err); } - return API.v1.failure(mountResult({ id, error: err })); + throw err; } }, }, From c82810854ecd4b2806bbed8738652294ea6830eb Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Tue, 30 Dec 2025 11:34:06 -0300 Subject: [PATCH 13/15] fix(api): return structured error responses instead of throwing exceptions in method calls --- apps/meteor/app/api/server/v1/misc.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/api/server/v1/misc.ts b/apps/meteor/app/api/server/v1/misc.ts index 4813eec93f16c..403ffcc29b4ce 100644 --- a/apps/meteor/app/api/server/v1/misc.ts +++ b/apps/meteor/app/api/server/v1/misc.ts @@ -526,7 +526,7 @@ API.v1.addRoute( Meteor._debug(`Exception while invoking method ${method}`, err); } - throw err; + return API.v1.failure(mountResult({ id, error: err })); } }, }, @@ -581,7 +581,7 @@ API.v1.addRoute( if (settings.get('Log_Level') === '2') { Meteor._debug(`Exception while invoking method ${method}`, err); } - throw err; + return API.v1.failure(mountResult({ id, error: err })); } }, }, From fe128b7459cad05fcc754e4fecb9f6a3fc2414b6 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Tue, 30 Dec 2025 11:39:14 -0300 Subject: [PATCH 14/15] refactor(api): update starMessage method to accept user object instead of userId for improved clarity and deprecate starMessage method --- apps/meteor/app/api/server/v1/chat.ts | 4 ++-- .../app/message-star/server/starMessage.ts | 22 ++++++++++--------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/apps/meteor/app/api/server/v1/chat.ts b/apps/meteor/app/api/server/v1/chat.ts index a2b5f93d1b0e3..25978aefce3e8 100644 --- a/apps/meteor/app/api/server/v1/chat.ts +++ b/apps/meteor/app/api/server/v1/chat.ts @@ -456,7 +456,7 @@ API.v1.addRoute( throw new Meteor.Error('error-message-not-found', 'The provided "messageId" does not match any existing message.'); } - await starMessage(this.userId, { + await starMessage(this.user, { _id: msg._id, rid: msg.rid, starred: true, @@ -478,7 +478,7 @@ API.v1.addRoute( throw new Meteor.Error('error-message-not-found', 'The provided "messageId" does not match any existing message.'); } - await starMessage(this.userId, { + await starMessage(this.user, { _id: msg._id, rid: msg.rid, starred: false, diff --git a/apps/meteor/app/message-star/server/starMessage.ts b/apps/meteor/app/message-star/server/starMessage.ts index 96b342f8bfa69..38cfc648af04d 100644 --- a/apps/meteor/app/message-star/server/starMessage.ts +++ b/apps/meteor/app/message-star/server/starMessage.ts @@ -1,11 +1,12 @@ import { Apps, AppEvents } from '@rocket.chat/apps'; -import type { IMessage } from '@rocket.chat/core-typings'; +import type { IMessage, IUser } from '@rocket.chat/core-typings'; import type { ServerMethods } from '@rocket.chat/ddp-client'; import { Messages, Subscriptions, Rooms } from '@rocket.chat/models'; import { Meteor } from 'meteor/meteor'; import { canAccessRoomAsync, roomAccessAttributes } from '../../authorization/server'; import { isTheLastMessage } from '../../lib/server/functions/isTheLastMessage'; +import { methodDeprecationLogger } from '../../lib/server/lib/deprecationWarningLogger'; import { notifyOnRoomChangedById, notifyOnMessageChange } from '../../lib/server/lib/notifyListener'; import { settings } from '../../settings/server'; @@ -16,7 +17,7 @@ declare module '@rocket.chat/ddp-client' { } } -export const starMessage = async (userId: string, message: Pick & { starred: boolean }): Promise => { +export const starMessage = async (user: IUser, message: Pick & { starred: boolean }): Promise => { if (!settings.get('Message_AllowStarring')) { throw new Meteor.Error('error-action-not-allowed', 'Message starring not allowed', { method: 'starMessage', @@ -24,7 +25,7 @@ export const starMessage = async (userId: string, message: Pick({ async starMessage(message) { - const uid = Meteor.userId(); + methodDeprecationLogger.method('starMessage', '9.0.0', '/v1/chat.starMessage'); + const user = (await Meteor.userAsync()) as IUser; - if (!uid) { + if (!user) { throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'starMessage', }); } - return starMessage(uid, message); + return starMessage(user, message); }, }); From d50a1692a8a5a5cf00fde2cf155b50c2ad879b19 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Tue, 30 Dec 2025 11:42:54 -0300 Subject: [PATCH 15/15] refactor(api): remove wrong user validation for getChannelHistory --- apps/meteor/app/lib/server/methods/getChannelHistory.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/apps/meteor/app/lib/server/methods/getChannelHistory.ts b/apps/meteor/app/lib/server/methods/getChannelHistory.ts index e0a2a844a75eb..cd2096a82a1a9 100644 --- a/apps/meteor/app/lib/server/methods/getChannelHistory.ts +++ b/apps/meteor/app/lib/server/methods/getChannelHistory.ts @@ -48,10 +48,6 @@ export const getChannelHistory = async ({ }): Promise => { check(rid, String); - if (!Meteor.userId()) { - throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'getChannelHistory' }); - } - if (!fromUserId) { return false; }