diff --git a/.changeset/weak-cobras-fry.md b/.changeset/weak-cobras-fry.md new file mode 100644 index 0000000000000..01d4d892188b6 --- /dev/null +++ b/.changeset/weak-cobras-fry.md @@ -0,0 +1,6 @@ +--- +'@rocket.chat/apps-engine': patch +'@rocket.chat/meteor': patch +--- + +Fixes an issue where some error objects sent to apps' method calls would only contain the message '[object Object]' diff --git a/packages/apps-engine/deno-runtime/lib/accessors/formatResponseErrorHandler.ts b/packages/apps-engine/deno-runtime/lib/accessors/formatResponseErrorHandler.ts new file mode 100644 index 0000000000000..6840c3ab5baa3 --- /dev/null +++ b/packages/apps-engine/deno-runtime/lib/accessors/formatResponseErrorHandler.ts @@ -0,0 +1,14 @@ +import { ErrorObject } from 'jsonrpc-lite'; + +// deno-lint-ignore no-explicit-any -- that is the type we get from `catch` +export const formatErrorResponse = (error: any): Error => { + if (error instanceof ErrorObject || typeof error?.error?.message === 'string') { + return new Error(error.error.message); + } + + if (error instanceof Error) { + return error; + } + + return new Error('An unknown error occurred', { cause: error }); +}; diff --git a/packages/apps-engine/deno-runtime/lib/accessors/http.ts b/packages/apps-engine/deno-runtime/lib/accessors/http.ts index 1a5e7098ef376..78703a67f3c1a 100644 --- a/packages/apps-engine/deno-runtime/lib/accessors/http.ts +++ b/packages/apps-engine/deno-runtime/lib/accessors/http.ts @@ -4,6 +4,7 @@ import type { IRead } from '@rocket.chat/apps-engine/definition/accessors/IRead. import * as Messenger from '../messenger.ts'; import { AppObjectRegistry } from '../../AppObjectRegistry.ts'; +import { formatErrorResponse } from './formatResponseErrorHandler.ts'; type RequestMethod = 'get' | 'post' | 'put' | 'head' | 'delete' | 'patch'; @@ -76,7 +77,7 @@ export class Http implements IHttp { url, request, }], - }); + }).catch((error) => { throw formatErrorResponse(error); }); for (const handler of this.httpExtender.getPreResponseHandlers()) { response = await handler.executePreHttpResponse(response as IHttpResponse, this.read, this.persistence); diff --git a/packages/apps-engine/deno-runtime/lib/accessors/mod.ts b/packages/apps-engine/deno-runtime/lib/accessors/mod.ts index 4479a2fb6a14e..ee1c1d530108d 100644 --- a/packages/apps-engine/deno-runtime/lib/accessors/mod.ts +++ b/packages/apps-engine/deno-runtime/lib/accessors/mod.ts @@ -22,6 +22,7 @@ import { ModifyCreator } from './modify/ModifyCreator.ts'; import { ModifyUpdater } from './modify/ModifyUpdater.ts'; import { ModifyExtender } from './modify/ModifyExtender.ts'; import { Notifier } from './notifier.ts'; +import { formatErrorResponse } from './formatResponseErrorHandler.ts'; const httpMethods = ['get', 'post', 'put', 'delete', 'head', 'options', 'patch'] as const; @@ -69,9 +70,7 @@ export class AppAccessors { params, }) .then((response) => response.result) - .catch((err) => { - throw new Error(err.error); - }); + .catch((err) => { throw formatErrorResponse(err) }); }, }, ) as T; diff --git a/packages/apps-engine/deno-runtime/lib/accessors/modify/ModifyCreator.ts b/packages/apps-engine/deno-runtime/lib/accessors/modify/ModifyCreator.ts index 38740aef1799e..b937103bab13b 100644 --- a/packages/apps-engine/deno-runtime/lib/accessors/modify/ModifyCreator.ts +++ b/packages/apps-engine/deno-runtime/lib/accessors/modify/ModifyCreator.ts @@ -28,6 +28,7 @@ import { UserBuilder } from '../builders/UserBuilder.ts'; import { AppVideoConference, VideoConferenceBuilder } from '../builders/VideoConferenceBuilder.ts'; import { AppObjectRegistry } from '../../../AppObjectRegistry.ts'; import { require } from '../../../lib/require.ts'; +import { formatErrorResponse } from '../formatResponseErrorHandler.ts'; const { UIHelper } = require('@rocket.chat/apps-engine/server/misc/UIHelper.js') as { UIHelper: typeof _UIHelper }; const { RoomType } = require('@rocket.chat/apps-engine/definition/rooms/RoomType.js') as { RoomType: typeof _RoomType }; @@ -59,15 +60,7 @@ export class ModifyCreator implements IModifyCreator { params, }) .then((response) => response.result) - .catch((err) => { - if (err instanceof Error) { - throw err; - } - if (err?.error?.message) { - throw new Error(err.error.message); - } - throw new Error(err.error); - }); + .catch((err) => { throw formatErrorResponse(err) }); }, }, ) as ILivechatCreator; @@ -83,15 +76,7 @@ export class ModifyCreator implements IModifyCreator { params, }) .then((response) => response.result) - .catch((err) => { - if (err instanceof Error) { - throw err; - } - if (err?.error?.message) { - throw new Error(err.error.message); - } - throw new Error(err.error); - }), + .catch((err) => { throw formatErrorResponse(err) }), }, ) as IUploadCreator; } @@ -106,15 +91,7 @@ export class ModifyCreator implements IModifyCreator { params, }) .then((response) => response.result) - .catch((err) => { - if (err instanceof Error) { - throw err; - } - if (err?.error?.message) { - throw new Error(err.error.message); - } - throw new Error(err.error); - }), + .catch((err) => { throw formatErrorResponse(err) }), }, ); } @@ -129,15 +106,7 @@ export class ModifyCreator implements IModifyCreator { params, }) .then((response) => response.result) - .catch((err) => { - if (err instanceof Error) { - throw err; - } - if (err?.error?.message) { - throw new Error(err.error.message); - } - throw new Error(err.error); - }), + .catch((err) => { throw formatErrorResponse(err) }), }, ); } @@ -236,7 +205,7 @@ export class ModifyCreator implements IModifyCreator { const response = await this.senderFn({ method: 'bridges:getUserBridge:doGetAppUser', params: ['APP_ID'], - }); + }).catch((err) => { throw formatErrorResponse(err) }); const appUser = response.result; @@ -255,7 +224,7 @@ export class ModifyCreator implements IModifyCreator { const response = await this.senderFn({ method: 'bridges:getMessageBridge:doCreate', params: [result, AppObjectRegistry.get('id')], - }); + }).catch((err) => { throw formatErrorResponse(err) }); return String(response.result); } @@ -277,7 +246,7 @@ export class ModifyCreator implements IModifyCreator { const response = await this.senderFn({ method: 'bridges:getLivechatBridge:doCreateMessage', params: [result, AppObjectRegistry.get('id')], - }); + }).catch((err) => { throw formatErrorResponse(err) }); return String(response.result); } @@ -311,7 +280,7 @@ export class ModifyCreator implements IModifyCreator { const response = await this.senderFn({ method: 'bridges:getRoomBridge:doCreate', params: [result, builder.getMembersToBeAddedUsernames(), AppObjectRegistry.get('id')], - }); + }).catch((err) => { throw formatErrorResponse(err) }); return String(response.result); } @@ -339,7 +308,7 @@ export class ModifyCreator implements IModifyCreator { const response = await this.senderFn({ method: 'bridges:getRoomBridge:doCreateDiscussion', params: [room, builder.getParentMessage(), builder.getReply(), builder.getMembersToBeAddedUsernames(), AppObjectRegistry.get('id')], - }); + }).catch((err) => { throw formatErrorResponse(err) }); return String(response.result); } @@ -362,7 +331,7 @@ export class ModifyCreator implements IModifyCreator { const response = await this.senderFn({ method: 'bridges:getVideoConferenceBridge:doCreate', params: [videoConference, AppObjectRegistry.get('id')], - }); + }).catch((err) => { throw formatErrorResponse(err) }); return String(response.result); } @@ -373,7 +342,7 @@ export class ModifyCreator implements IModifyCreator { const response = await this.senderFn({ method: 'bridges:getUserBridge:doCreate', params: [user, AppObjectRegistry.get('id')], - }); + }).catch((err) => { throw formatErrorResponse(err) }); return String(response.result); } diff --git a/packages/apps-engine/deno-runtime/lib/accessors/modify/ModifyExtender.ts b/packages/apps-engine/deno-runtime/lib/accessors/modify/ModifyExtender.ts index d9e9678e376d1..1a7fa861814a4 100644 --- a/packages/apps-engine/deno-runtime/lib/accessors/modify/ModifyExtender.ts +++ b/packages/apps-engine/deno-runtime/lib/accessors/modify/ModifyExtender.ts @@ -14,6 +14,7 @@ import { MessageExtender } from '../extenders/MessageExtender.ts'; import { RoomExtender } from '../extenders/RoomExtender.ts'; import { VideoConferenceExtender } from '../extenders/VideoConferenceExtend.ts'; import { require } from '../../../lib/require.ts'; +import { formatErrorResponse } from '../formatResponseErrorHandler.ts'; const { RocketChatAssociationModel } = require('@rocket.chat/apps-engine/definition/metadata/RocketChatAssociations.js') as { RocketChatAssociationModel: typeof _RocketChatAssociationModel; @@ -26,7 +27,7 @@ export class ModifyExtender implements IModifyExtender { const result = await this.senderFn({ method: 'bridges:getMessageBridge:doGetById', params: [messageId, AppObjectRegistry.get('id')], - }); + }).catch((err) => { throw formatErrorResponse(err) }); const msg = result.result as IMessage; @@ -40,7 +41,7 @@ export class ModifyExtender implements IModifyExtender { const result = await this.senderFn({ method: 'bridges:getRoomBridge:doGetById', params: [roomId, AppObjectRegistry.get('id')], - }); + }).catch((err) => { throw formatErrorResponse(err) }); const room = result.result as IRoom; @@ -53,7 +54,7 @@ export class ModifyExtender implements IModifyExtender { const result = await this.senderFn({ method: 'bridges:getVideoConferenceBridge:doGetById', params: [id, AppObjectRegistry.get('id')], - }); + }).catch((err) => { throw formatErrorResponse(err) }); const call = result.result as VideoConference; @@ -68,7 +69,7 @@ export class ModifyExtender implements IModifyExtender { await this.senderFn({ method: 'bridges:getMessageBridge:doUpdate', params: [(extender as IMessageExtender).getMessage(), AppObjectRegistry.get('id')], - }); + }).catch((err) => { throw formatErrorResponse(err) }); break; case RocketChatAssociationModel.ROOM: await this.senderFn({ @@ -78,13 +79,13 @@ export class ModifyExtender implements IModifyExtender { (extender as IRoomExtender).getUsernamesOfMembersBeingAdded(), AppObjectRegistry.get('id'), ], - }); + }).catch((err) => { throw formatErrorResponse(err) }); break; case RocketChatAssociationModel.VIDEO_CONFERENCE: await this.senderFn({ method: 'bridges:getVideoConferenceBridge:doUpdate', params: [(extender as IVideoConferenceExtender).getVideoConference(), AppObjectRegistry.get('id')], - }); + }).catch((err) => { throw formatErrorResponse(err) }); break; default: throw new Error('Invalid extender passed to the ModifyExtender.finish function.'); diff --git a/packages/apps-engine/deno-runtime/lib/accessors/modify/ModifyUpdater.ts b/packages/apps-engine/deno-runtime/lib/accessors/modify/ModifyUpdater.ts index 6664f86092fa5..0a720ec8a2ede 100644 --- a/packages/apps-engine/deno-runtime/lib/accessors/modify/ModifyUpdater.ts +++ b/packages/apps-engine/deno-runtime/lib/accessors/modify/ModifyUpdater.ts @@ -18,6 +18,7 @@ import { RoomBuilder } from '../builders/RoomBuilder.ts'; import { AppObjectRegistry } from '../../../AppObjectRegistry.ts'; import { require } from '../../../lib/require.ts'; +import { formatErrorResponse } from '../formatResponseErrorHandler.ts'; const { UIHelper } = require('@rocket.chat/apps-engine/server/misc/UIHelper.js') as { UIHelper: typeof _UIHelper }; const { RoomType } = require('@rocket.chat/apps-engine/definition/rooms/RoomType.js') as { RoomType: typeof _RoomType }; @@ -38,9 +39,7 @@ export class ModifyUpdater implements IModifyUpdater { params, }) .then((response) => response.result) - .catch((err) => { - throw new Error(err.error); - }), + .catch((err) => { throw formatErrorResponse(err) }), }, ) as ILivechatUpdater; } @@ -55,9 +54,7 @@ export class ModifyUpdater implements IModifyUpdater { params, }) .then((response) => response.result) - .catch((err) => { - throw new Error(err.error); - }), + .catch((err) => { throw formatErrorResponse(err) }), }, ) as IUserUpdater; } @@ -66,7 +63,7 @@ export class ModifyUpdater implements IModifyUpdater { const response = await this.senderFn({ method: 'bridges:getMessageBridge:doGetById', params: [messageId, AppObjectRegistry.get('id')], - }); + }).catch((err) => { throw formatErrorResponse(err) }); const builder = new MessageBuilder(response.result as IMessage); @@ -79,7 +76,7 @@ export class ModifyUpdater implements IModifyUpdater { const response = await this.senderFn({ method: 'bridges:getRoomBridge:doGetById', params: [roomId, AppObjectRegistry.get('id')], - }); + }).catch((err) => { throw formatErrorResponse(err) }); return new RoomBuilder(response.result as IRoom); } @@ -115,7 +112,7 @@ export class ModifyUpdater implements IModifyUpdater { await this.senderFn({ method: 'bridges:getMessageBridge:doUpdate', params: [changes, AppObjectRegistry.get('id')], - }); + }).catch((err) => { throw formatErrorResponse(err) }); } private async _finishRoom(builder: RoomBuilder): Promise { @@ -148,6 +145,6 @@ export class ModifyUpdater implements IModifyUpdater { await this.senderFn({ method: 'bridges:getRoomBridge:doUpdate', params: [changes, builder.getMembersToBeAddedUsernames(), AppObjectRegistry.get('id')], - }); + }).catch((err) => { throw formatErrorResponse(err) }); } } diff --git a/packages/apps-engine/deno-runtime/lib/accessors/notifier.ts b/packages/apps-engine/deno-runtime/lib/accessors/notifier.ts index 81868c2a85b06..1a85cc12b579f 100644 --- a/packages/apps-engine/deno-runtime/lib/accessors/notifier.ts +++ b/packages/apps-engine/deno-runtime/lib/accessors/notifier.ts @@ -8,6 +8,7 @@ import { MessageBuilder } from './builders/MessageBuilder.ts'; import { AppObjectRegistry } from '../../AppObjectRegistry.ts'; import * as Messenger from '../messenger.ts'; import { require } from '../require.ts'; +import { formatErrorResponse } from './formatResponseErrorHandler.ts'; const { TypingScope } = require('@rocket.chat/apps-engine/definition/accessors/INotifier.js') as { TypingScope: typeof _TypingScope; @@ -65,11 +66,19 @@ export class Notifier implements INotifier { await this.senderFn({ method: `bridges:getMessageBridge:${method}`, params, + }).catch((err) => { + throw formatErrorResponse(err); }); } private async getAppUser(): Promise { - const response = await this.senderFn({ method: 'bridges:getUserBridge:doGetAppUser', params: [AppObjectRegistry.get('id')] }); + const response = await this.senderFn({ + method: 'bridges:getUserBridge:doGetAppUser', + params: [AppObjectRegistry.get('id')], + }).catch((err) => { + throw formatErrorResponse(err); + }); + return response.result; } } diff --git a/packages/apps-engine/deno-runtime/lib/accessors/tests/ModifyCreator.test.ts b/packages/apps-engine/deno-runtime/lib/accessors/tests/ModifyCreator.test.ts index 43bb7d413dcfe..d88690a77dbfa 100644 --- a/packages/apps-engine/deno-runtime/lib/accessors/tests/ModifyCreator.test.ts +++ b/packages/apps-engine/deno-runtime/lib/accessors/tests/ModifyCreator.test.ts @@ -151,7 +151,7 @@ describe('ModifyCreator', () => { name: 'Visitor Name', }), Error, - '[object Object]', + 'An unknown error occurred', ); }); @@ -176,7 +176,7 @@ describe('ModifyCreator', () => { const modifyCreator = new ModifyCreator(failingSenderFn); const uploadCreator = modifyCreator.getUploadCreator(); - await assertRejects(() => uploadCreator.uploadBuffer(new Uint8Array([1, 2, 3]), 'image/png'), Error, '[object Object]'); + await assertRejects(() => uploadCreator.uploadBuffer(new Uint8Array([1, 2, 3]), 'image/png'), Error, 'An unknown error occurred'); }); it('throws an error when a proxy method of getEmailCreator fails', async () => { @@ -229,7 +229,7 @@ describe('ModifyCreator', () => { text: 'This is a test email.', }), Error, - '[object Object]', + 'An unknown error occurred', ); }); @@ -254,6 +254,6 @@ describe('ModifyCreator', () => { const modifyCreator = new ModifyCreator(failingSenderFn); const contactCreator = modifyCreator.getContactCreator(); - await assertRejects(() => contactCreator.addContactEmail('test-contact-id', 'test@example.com'), Error, '[object Object]'); + await assertRejects(() => contactCreator.addContactEmail('test-contact-id', 'test@example.com'), Error, 'An unknown error occurred'); }); }); diff --git a/packages/apps-engine/deno-runtime/lib/accessors/tests/ModifyExtender.test.ts b/packages/apps-engine/deno-runtime/lib/accessors/tests/ModifyExtender.test.ts index 66e14a1680b49..56e5a32548231 100644 --- a/packages/apps-engine/deno-runtime/lib/accessors/tests/ModifyExtender.test.ts +++ b/packages/apps-engine/deno-runtime/lib/accessors/tests/ModifyExtender.test.ts @@ -1,9 +1,11 @@ // deno-lint-ignore-file no-explicit-any import { afterAll, beforeEach, describe, it } from 'https://deno.land/std@0.203.0/testing/bdd.ts'; -import { assertSpyCall, spy } from 'https://deno.land/std@0.203.0/testing/mock.ts'; +import { assertSpyCall, spy, stub } from 'https://deno.land/std@0.203.0/testing/mock.ts'; +import { assertRejects } from 'https://deno.land/std@0.203.0/assert/mod.ts'; import { AppObjectRegistry } from '../../../AppObjectRegistry.ts'; import { ModifyExtender } from '../modify/ModifyExtender.ts'; +import jsonrpc from 'jsonrpc-lite'; describe('ModifyExtender', () => { let extender: ModifyExtender; @@ -117,4 +119,110 @@ describe('ModifyExtender', () => { _spy.restore(); }); + + describe('Error Handling', () => { + describe('extendMessage', () => { + it('throws an instance of Error when senderFn throws an error', async () => { + const _stub = stub(extender, 'senderFn' as keyof ModifyExtender, () => Promise.reject(new Error('unit-test-error')) as any); + + await assertRejects(() => extender.extendMessage('message-id', { _id: 'user-id' } as any), Error, 'unit-test-error'); + + _stub.restore(); + }); + + it('throws an instance of Error when senderFn throws a jsonrpc error', async () => { + const _stub = stub(extender, 'senderFn' as keyof ModifyExtender, () => Promise.reject(jsonrpc.error('unit-test-error', new jsonrpc.JsonRpcError('unit-test-error', 1000))) as any); + + await assertRejects(() => extender.extendMessage('message-id', { _id: 'user-id' } as any), Error, 'unit-test-error'); + + _stub.restore(); + }); + + it('throws an instance of Error when senderFn throws an unknown value', async () => { + const _stub = stub(extender, 'senderFn' as keyof ModifyExtender, () => Promise.reject({}) as any); + + await assertRejects(() => extender.extendMessage('message-id', { _id: 'user-id' } as any), Error, 'An unknown error occurred'); + + _stub.restore(); + }); + }); + + describe('extendRoom', () => { + it('throws an instance of Error when senderFn throws an error', async () => { + const _stub = stub(extender, 'senderFn' as keyof ModifyExtender, () => Promise.reject(new Error('unit-test-error')) as any); + + await assertRejects(() => extender.extendRoom('room-id', { _id: 'user-id' } as any), Error, 'unit-test-error'); + + _stub.restore(); + }); + + it('throws an instance of Error when senderFn throws a jsonrpc error', async () => { + const _stub = stub(extender, 'senderFn' as keyof ModifyExtender, () => Promise.reject(jsonrpc.error('unit-test-error', new jsonrpc.JsonRpcError('unit-test-error', 1000))) as any); + + await assertRejects(() => extender.extendRoom('room-id', { _id: 'user-id' } as any), Error, 'unit-test-error'); + + _stub.restore(); + }); + + it('throws an instance of Error when senderFn throws an unknown value', async () => { + const _stub = stub(extender, 'senderFn' as keyof ModifyExtender, () => Promise.reject({}) as any); + + await assertRejects(() => extender.extendRoom('room-id', { _id: 'user-id' } as any), Error, 'An unknown error occurred'); + + _stub.restore(); + }); + }); + + describe('extendVideoConference', () => { + it('throws an instance of Error when senderFn throws an error', async () => { + const _stub = stub(extender, 'senderFn' as keyof ModifyExtender, () => Promise.reject(new Error('unit-test-error')) as any); + + await assertRejects(() => extender.extendVideoConference('video-conference-id'), Error, 'unit-test-error'); + + _stub.restore(); + }); + + it('throws an instance of Error when senderFn throws a jsonrpc error', async () => { + const _stub = stub(extender, 'senderFn' as keyof ModifyExtender, () => Promise.reject(jsonrpc.error('unit-test-error', new jsonrpc.JsonRpcError('unit-test-error', 1000))) as any); + + await assertRejects(() => extender.extendVideoConference('video-conference-id'), Error, 'unit-test-error'); + + _stub.restore(); + }); + + it('throws an instance of Error when senderFn throws an unknown value', async () => { + const _stub = stub(extender, 'senderFn' as keyof ModifyExtender, () => Promise.reject({}) as any); + + await assertRejects(() => extender.extendVideoConference('video-conference-id'), Error, 'An unknown error occurred'); + + _stub.restore(); + }); + }); + + describe('finish', () => { + it('throws an instance of Error when senderFn throws an error', async () => { + const _stub = stub(extender, 'senderFn' as keyof ModifyExtender, () => Promise.reject(new Error('unit-test-error')) as any); + + await assertRejects(() => extender.finish({ kind: 'message', getMessage: () => ({})} as any), Error, 'unit-test-error'); + + _stub.restore(); + }); + + it('throws an instance of Error when senderFn throws a jsonrpc error', async () => { + const _stub = stub(extender, 'senderFn' as keyof ModifyExtender, () => Promise.reject(jsonrpc.error('unit-test-error', new jsonrpc.JsonRpcError('unit-test-error', 1000))) as any); + + await assertRejects(() => extender.finish({ kind: 'message', getMessage: () => ({})} as any), Error, 'unit-test-error'); + + _stub.restore(); + }); + + it('throws an instance of Error when senderFn throws an unknown value', async () => { + const _stub = stub(extender, 'senderFn' as keyof ModifyExtender, () => Promise.reject({}) as any); + + await assertRejects(() => extender.finish({ kind: 'message', getMessage: () => ({})} as any), Error, 'An unknown error occurred'); + + _stub.restore(); + }); + }); + }); }); diff --git a/packages/apps-engine/deno-runtime/lib/accessors/tests/ModifyUpdater.test.ts b/packages/apps-engine/deno-runtime/lib/accessors/tests/ModifyUpdater.test.ts index a93656a84c8a7..fa1af8f5ebe32 100644 --- a/packages/apps-engine/deno-runtime/lib/accessors/tests/ModifyUpdater.test.ts +++ b/packages/apps-engine/deno-runtime/lib/accessors/tests/ModifyUpdater.test.ts @@ -1,11 +1,12 @@ // deno-lint-ignore-file no-explicit-any import { afterAll, beforeEach, describe, it } from 'https://deno.land/std@0.203.0/testing/bdd.ts'; -import { assertSpyCall, spy } from 'https://deno.land/std@0.203.0/testing/mock.ts'; -import { assertEquals } from 'https://deno.land/std@0.203.0/assert/mod.ts'; +import { assertSpyCall, spy, stub } from 'https://deno.land/std@0.203.0/testing/mock.ts'; +import { assertEquals, assertRejects } from 'https://deno.land/std@0.203.0/assert/mod.ts'; import { AppObjectRegistry } from '../../../AppObjectRegistry.ts'; import { ModifyUpdater } from '../modify/ModifyUpdater.ts'; import { RoomBuilder } from '../builders/RoomBuilder.ts'; +import jsonrpc from 'jsonrpc-lite'; describe('ModifyUpdater', () => { let modifyUpdater: ModifyUpdater; @@ -126,4 +127,96 @@ describe('ModifyUpdater', () => { params: [{ id: '123' }, 'close it!'], }); }); + + describe('Error Handling', () => { + describe('message', () => { + it('throws an instance of Error when senderFn throws an error', async () => { + const _stub = stub(modifyUpdater, 'senderFn' as keyof ModifyUpdater, () => Promise.reject(new Error('unit-test-error')) as any); + + await assertRejects(() => modifyUpdater.message('message-id', { _id: 'user-id' } as any), Error, 'unit-test-error'); + + _stub.restore(); + }); + + it('throws an instance of Error when senderFn throws a jsonrpc error', async () => { + const _stub = stub(modifyUpdater, 'senderFn' as keyof ModifyUpdater, () => Promise.reject(jsonrpc.error('unit-test-error', new jsonrpc.JsonRpcError('unit-test-error', 1000))) as any); + + await assertRejects(() => modifyUpdater.message('message-id', { _id: 'user-id' } as any), Error, 'unit-test-error'); + + _stub.restore(); + }); + + it('throws an instance of Error when senderFn throws an unknown value', async () => { + const _stub = stub(modifyUpdater, 'senderFn' as keyof ModifyUpdater, () => Promise.reject({}) as any); + + await assertRejects(() => modifyUpdater.message('message-id', { _id: 'user-id' } as any), Error, 'An unknown error occurred'); + + _stub.restore(); + }); + }); + + describe('room', () => { + it('throws an instance of Error when senderFn throws an error', async () => { + const _stub = stub(modifyUpdater, 'senderFn' as keyof ModifyUpdater, () => Promise.reject(new Error('unit-test-error')) as any); + + await assertRejects(() => modifyUpdater.room('room-id', { _id: 'user-id' } as any), Error, 'unit-test-error'); + + _stub.restore(); + }); + + it('throws an instance of Error when senderFn throws a jsonrpc error', async () => { + const _stub = stub(modifyUpdater, 'senderFn' as keyof ModifyUpdater, () => Promise.reject(jsonrpc.error('unit-test-error', new jsonrpc.JsonRpcError('unit-test-error', 1000))) as any); + + await assertRejects(() => modifyUpdater.room('room-id', { _id: 'user-id' } as any), Error, 'unit-test-error'); + + _stub.restore(); + }); + + it('throws an instance of Error when senderFn throws an unknown value', async () => { + const _stub = stub(modifyUpdater, 'senderFn' as keyof ModifyUpdater, () => Promise.reject({}) as any); + + await assertRejects(() => modifyUpdater.room('room-id', { _id: 'user-id' } as any), Error, 'An unknown error occurred'); + + _stub.restore(); + }); + }); + + describe('finish', () => { + const messageUpdater = { + kind: 'message', + getMessage: () => ({ + id: 'message-id', + sender: { id: 'sender-id' }, + }), + getChanges: () => ({ + id: 'message-id', + sender: { id: 'sender-id' }, + }), + } as any; + + it('throws an instance of Error when senderFn throws an error', async () => { + const _stub = stub(modifyUpdater, 'senderFn' as keyof ModifyUpdater, () => Promise.reject(new Error('unit-test-error')) as any); + + await assertRejects(() => modifyUpdater.finish(messageUpdater), Error, 'unit-test-error'); + + _stub.restore(); + }); + + it('throws an instance of Error when senderFn throws a jsonrpc error', async () => { + const _stub = stub(modifyUpdater, 'senderFn' as keyof ModifyUpdater, () => Promise.reject(jsonrpc.error('unit-test-error', new jsonrpc.JsonRpcError('unit-test-error', 1000))) as any); + + await assertRejects(() => modifyUpdater.finish(messageUpdater), Error, 'unit-test-error'); + + _stub.restore(); + }); + + it('throws an instance of Error when senderFn throws an unknown value', async () => { + const _stub = stub(modifyUpdater, 'senderFn' as keyof ModifyUpdater, () => Promise.reject({}) as any); + + await assertRejects(() => modifyUpdater.finish(messageUpdater), Error, 'An unknown error occurred'); + + _stub.restore(); + }); + }); + }); }); diff --git a/packages/apps-engine/deno-runtime/lib/accessors/tests/formatResponseErrorHandler.test.ts b/packages/apps-engine/deno-runtime/lib/accessors/tests/formatResponseErrorHandler.test.ts new file mode 100644 index 0000000000000..74683fa1c2d77 --- /dev/null +++ b/packages/apps-engine/deno-runtime/lib/accessors/tests/formatResponseErrorHandler.test.ts @@ -0,0 +1,216 @@ +// deno-lint-ignore-file no-explicit-any +import { assertEquals, assertInstanceOf, assertStrictEquals } from 'https://deno.land/std@0.203.0/assert/mod.ts'; +import { describe, it } from 'https://deno.land/std@0.203.0/testing/bdd.ts'; +import * as jsonrpc from 'jsonrpc-lite'; + +import { formatErrorResponse } from '../formatResponseErrorHandler.ts'; + +describe('formatErrorResponse', () => { + describe('JSON-RPC ErrorObject handling', () => { + it('formats ErrorObject instances correctly', () => { + const errorObject = jsonrpc.error('test-id', new jsonrpc.JsonRpcError('Test error message', 1000)); + const result = formatErrorResponse(errorObject); + + assertInstanceOf(result, Error); + assertEquals(result.message, 'Test error message'); + }); + + it('formats objects with error.message structure', () => { + const errorLikeObject = { + error: { + message: 'Custom error message', + code: 404, + }, + }; + const result = formatErrorResponse(errorLikeObject); + + assertInstanceOf(result, Error); + assertEquals(result.message, 'Custom error message'); + }); + + it('handles nested error objects with complex structure', () => { + const complexError = { + error: { + message: 'Database connection failed', + details: { + host: 'localhost', + port: 5432, + }, + }, + id: 'req-123', + }; + const result = formatErrorResponse(complexError); + + assertInstanceOf(result, Error); + assertEquals(result.message, 'Database connection failed'); + }); + + it('handles error objects with empty message', () => { + const emptyMessageError = { + error: { + message: '', + code: 500, + }, + }; + const result = formatErrorResponse(emptyMessageError); + + assertInstanceOf(result, Error); + assertEquals(result.message, ''); + }); + }); + + describe('Error instance passthrough', () => { + it('returns existing Error instances unchanged', () => { + const originalError = new Error('Original error message'); + const result = formatErrorResponse(originalError); + + assertStrictEquals(result, originalError); + assertEquals(result.message, 'Original error message'); + }); + + it('returns custom Error subclasses unchanged', () => { + class CustomError extends Error { + constructor(message: string, public code: number) { + super(message); + this.name = 'CustomError'; + } + } + + const customError = new CustomError('Custom error', 404); + const result = formatErrorResponse(customError); + + assertStrictEquals(result, customError); + assertEquals(result.message, 'Custom error'); + assertEquals((result as CustomError).code, 404); + }); + + it('handles Error instances with additional properties', () => { + const errorWithProps = new Error('Error with props') as any; + errorWithProps.statusCode = 500; + errorWithProps.details = { reason: 'timeout' }; + + const result = formatErrorResponse(errorWithProps); + + assertStrictEquals(result, errorWithProps); + assertEquals(result.message, 'Error with props'); + assertEquals((result as any).statusCode, 500); + }); + }); + + describe('Unknown error handling', () => { + it('wraps string errors with default message and cause', () => { + const stringError = 'Simple string error'; + const result = formatErrorResponse(stringError); + + assertInstanceOf(result, Error); + assertEquals(result.message, 'An unknown error occurred'); + assertEquals(result.cause, stringError); + }); + + it('wraps number errors with default message and cause', () => { + const numberError = 404; + const result = formatErrorResponse(numberError); + + assertInstanceOf(result, Error); + assertEquals(result.message, 'An unknown error occurred'); + assertEquals(result.cause, numberError); + }); + + it('wraps boolean errors with default message and cause', () => { + const booleanError = false; + const result = formatErrorResponse(booleanError); + + assertInstanceOf(result, Error); + assertEquals(result.message, 'An unknown error occurred'); + assertEquals(result.cause, booleanError); + }); + + it('wraps null with default message and cause', () => { + const result = formatErrorResponse(null); + + assertInstanceOf(result, Error); + assertEquals(result.message, 'An unknown error occurred'); + assertEquals(result.cause, null); + }); + + it('wraps undefined with default message and cause', () => { + const result = formatErrorResponse(undefined); + + assertInstanceOf(result, Error); + assertEquals(result.message, 'An unknown error occurred'); + assertEquals(result.cause, undefined); + }); + + it('wraps arrays with default message and cause', () => { + const arrayError = ['error', 'details']; + const result = formatErrorResponse(arrayError); + + assertInstanceOf(result, Error); + assertEquals(result.message, 'An unknown error occurred'); + assertEquals(result.cause, arrayError); + }); + + it('wraps functions with default message and cause', () => { + const functionError = () => 'error'; + const result = formatErrorResponse(functionError); + + assertInstanceOf(result, Error); + assertEquals(result.message, 'An unknown error occurred'); + assertEquals(result.cause, functionError); + }); + + it('wraps plain objects without error.message with default message and cause', () => { + const plainObject = { + status: 'failed', + reason: 'timeout', + data: { id: 123 }, + }; + const result = formatErrorResponse(plainObject); + + assertInstanceOf(result, Error); + assertEquals(result.message, 'An unknown error occurred'); + assertEquals(result.cause, plainObject); + }); + + it('wraps objects with error property but no message with default message and cause', () => { + const errorObjectNoMessage = { + error: { + code: 500, + details: 'Internal server error', + }, + }; + const result = formatErrorResponse(errorObjectNoMessage); + + assertInstanceOf(result, Error); + assertEquals(result.message, 'An unknown error occurred'); + assertEquals(result.cause, errorObjectNoMessage); + }); + }); + + it('ensures all returned values are proper Error instances', () => { + const testCases = [ + 'string error', + 123, + null, + undefined, + { error: { message: 'test' } }, + new Error('test'), + { plain: 'object' }, + ]; + + for (const testCase of testCases) { + const result = formatErrorResponse(testCase); + assertInstanceOf(result, Error, `Failed for input: ${JSON.stringify(testCase)}`); + } + }); + + it('prevents "[object Object]" error messages for plain objects', () => { + const plainObject = { status: 'error', code: 500 }; + const result = formatErrorResponse(plainObject); + + assertInstanceOf(result, Error); + assertEquals(result.message, 'An unknown error occurred'); + // Ensure the message is not "[object Object]" + assertEquals(result.message !== '[object Object]', true); + }); +}); diff --git a/packages/apps-engine/deno-runtime/lib/accessors/tests/http.test.ts b/packages/apps-engine/deno-runtime/lib/accessors/tests/http.test.ts new file mode 100644 index 0000000000000..88392dec774cc --- /dev/null +++ b/packages/apps-engine/deno-runtime/lib/accessors/tests/http.test.ts @@ -0,0 +1,164 @@ +// deno-lint-ignore-file no-explicit-any +import { assertRejects } from 'https://deno.land/std@0.203.0/assert/mod.ts'; +import { beforeEach, describe, it, afterAll } from 'https://deno.land/std@0.203.0/testing/bdd.ts'; + +import { Http } from '../http.ts'; +import { AppObjectRegistry } from '../../../AppObjectRegistry.ts'; +import { stub } from 'https://deno.land/std@0.203.0/testing/mock.ts'; + +describe('Http accessor error handling integration', () => { + let http: Http; + + beforeEach(() => { + AppObjectRegistry.clear(); + AppObjectRegistry.set('id', 'test-app-id'); + + const mockHttpExtend = { + getDefaultHeaders: () => new Map(), + getDefaultParams: () => new Map(), + getPreRequestHandlers: () => [], + getPreResponseHandlers: () => [], + }; + + const mockRead = {}; + const mockPersistence = {}; + + http = new Http(mockRead as any, mockPersistence as any, mockHttpExtend as any, () => Promise.resolve({}) as any); + }); + + afterAll(() => { + AppObjectRegistry.clear(); + }); + + describe('HTTP method error handling', () => { + it('formats JSON-RPC errors correctly for GET requests', async () => { + const _stub = stub(http, 'senderFn' as keyof Http, () => + Promise.reject({ + error: { + message: 'HTTP GET request failed', + code: 404, + }, + }), + ); + + await assertRejects(() => http.get('https://api.example.com/data'), Error, 'HTTP GET request failed'); + + _stub.restore(); + }); + + it('formats JSON-RPC errors correctly for POST requests', async () => { + const _stub = stub(http, 'senderFn' as keyof Http, () => + Promise.reject({ + error: { + message: 'HTTP POST request validation failed', + code: 400, + }, + }), + ); + + await assertRejects( + () => http.post('https://api.example.com/create', { data: { name: 'test' } }), + Error, + 'HTTP POST request validation failed', + ); + + _stub.restore(); + }); + + it('formats JSON-RPC errors correctly for PUT requests', async () => { + const _stub = stub(http, 'senderFn' as keyof Http, () => + Promise.reject({ + error: { + message: 'HTTP PUT request unauthorized', + code: 401, + }, + }), + ); + + await assertRejects( + () => http.put('https://api.example.com/update/123', { data: { name: 'updated' } }), + Error, + 'HTTP PUT request unauthorized', + ); + + _stub.restore(); + }); + + it('formats JSON-RPC errors correctly for DELETE requests', async () => { + const _stub = stub(http, 'senderFn' as keyof Http, () => + Promise.reject({ + error: { + message: 'HTTP DELETE request forbidden', + code: 403, + }, + }), + ); + + await assertRejects(() => http.del('https://api.example.com/delete/123'), Error, 'HTTP DELETE request forbidden'); + + _stub.restore(); + }); + + it('formats JSON-RPC errors correctly for PATCH requests', async () => { + const _stub = stub(http, 'senderFn' as keyof Http, () => + Promise.reject({ + error: { + message: 'HTTP PATCH request conflict', + code: 409, + }, + }), + ); + + await assertRejects( + () => http.patch('https://api.example.com/patch/123', { data: { status: 'active' } }), + Error, + 'HTTP PATCH request conflict', + ); + + _stub.restore(); + }); + }); + + describe('Error instance passthrough', () => { + it('passes through existing Error instances unchanged for HTTP requests', async () => { + const originalError = new Error('Network timeout error'); + const _stub = stub(http, 'senderFn' as keyof Http, () => Promise.reject(originalError)); + + await assertRejects(() => http.get('https://api.example.com/data'), Error, 'Network timeout error'); + + _stub.restore(); + }); + }); + + describe('Unknown error handling', () => { + it('wraps unknown object errors with default message for HTTP requests', async () => { + const unknownError = { + status: 'failed', + details: 'Something went wrong', + timestamp: Date.now(), + }; + const _stub = stub(http, 'senderFn' as keyof Http, () => Promise.reject(unknownError)); + + await assertRejects(() => http.get('https://api.example.com/data'), Error, 'An unknown error occurred'); + + _stub.restore(); + }); + + it('wraps string errors with default message for HTTP requests', async () => { + const stringError = 'Connection refused'; + const _stub = stub(http, 'senderFn' as keyof Http, () => Promise.reject(stringError)); + + await assertRejects(() => http.get('https://api.example.com/data'), Error, 'An unknown error occurred'); + + _stub.restore(); + }); + + it('wraps null/undefined errors with default message for HTTP requests', async () => { + const _stub = stub(http, 'senderFn' as keyof Http, () => Promise.reject(null)); + + await assertRejects(() => http.get('https://api.example.com/data'), Error, 'An unknown error occurred'); + + _stub.restore(); + }); + }); +}); diff --git a/packages/apps-engine/deno-runtime/lib/roomFactory.ts b/packages/apps-engine/deno-runtime/lib/roomFactory.ts index 902eba92cfc15..ac108399c1fd0 100644 --- a/packages/apps-engine/deno-runtime/lib/roomFactory.ts +++ b/packages/apps-engine/deno-runtime/lib/roomFactory.ts @@ -3,7 +3,7 @@ import type { AppManager } from '@rocket.chat/apps-engine/server/AppManager.ts'; import { AppAccessors } from './accessors/mod.ts'; import { Room } from './room.ts'; -import { JsonRpcError } from 'jsonrpc-lite'; +import { formatErrorResponse } from './accessors/formatResponseErrorHandler.ts'; const getMockAppManager = (senderFn: AppAccessors['senderFn']) => ({ getBridges: () => ({ @@ -13,7 +13,7 @@ const getMockAppManager = (senderFn: AppAccessors['senderFn']) => ({ method: 'bridges:getInternalBridge:doGetUsernamesOfRoomById', params: [roomId], }).then((result) => result.result).catch((err) => { - throw new JsonRpcError(`Error getting usernames of room: ${err}`, -32000); + throw formatErrorResponse(err); }); }, }),