diff --git a/packages/twenty-server/src/engine/metadata-modules/front-component/controllers/front-component.controller.ts b/packages/twenty-server/src/engine/metadata-modules/front-component/controllers/front-component.controller.ts index 22990a3149..c87e54d68c 100644 --- a/packages/twenty-server/src/engine/metadata-modules/front-component/controllers/front-component.controller.ts +++ b/packages/twenty-server/src/engine/metadata-modules/front-component/controllers/front-component.controller.ts @@ -1,6 +1,7 @@ import { Controller, Get, + Logger, Param, Res, UseFilters, @@ -15,6 +16,7 @@ import { FileStorageException, FileStorageExceptionCode, } from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception'; +import { setFileResponseHeaders } from 'src/engine/core-modules/file/utils/set-file-response-headers.utils'; import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator'; @@ -39,6 +41,8 @@ import { WorkspaceMigrationRunnerRestApiExceptionFilter } from 'src/engine/works WorkspaceMigrationRunnerRestApiExceptionFilter, ) export class FrontComponentController { + private readonly logger = new Logger(FrontComponentController.name); + constructor(private readonly frontComponentService: FrontComponentService) {} @Get(':frontComponentId') @@ -48,40 +52,58 @@ export class FrontComponentController { @Param('frontComponentId') frontComponentId: string, @AuthWorkspace() workspace: WorkspaceEntity, ) { - try { - const fileStream = - await this.frontComponentService.getBuiltComponentStream({ - frontComponentId, - workspaceId: workspace.id, - }); + const fileResponse = await this.frontComponentService + .getBuiltComponentPresignedUrlOrStream({ + frontComponentId, + workspaceId: workspace.id, + }) + .catch((error) => { + if (error instanceof FrontComponentException) { + throw error; + } - res.setHeader('Content-Type', 'application/javascript'); + if ( + error instanceof FileStorageException && + error.code === FileStorageExceptionCode.FILE_NOT_FOUND + ) { + throw new FrontComponentException( + 'Front component built file not found', + FrontComponentExceptionCode.FRONT_COMPONENT_NOT_FOUND, + ); + } - await pipeline(fileStream, res); - } catch (error) { - // Mid-stream error: client already received partial data, nothing to do - if (res.headersSent) { - return; - } + this.logger.error( + 'getBuiltComponentPresignedUrlOrStream failed unexpectedly', + { error }, + ); - if ( - error instanceof FileStorageException && - error.code === FileStorageExceptionCode.FILE_NOT_FOUND - ) { throw new FrontComponentException( - 'Front component built file not found', - FrontComponentExceptionCode.FRONT_COMPONENT_NOT_FOUND, + 'Error retrieving front component built file', + FrontComponentExceptionCode.FRONT_COMPONENT_NOT_READY, + ); + }); + + if (fileResponse.type === 'redirect') { + return res.redirect(fileResponse.presignedUrl); + } + + setFileResponseHeaders(res, fileResponse.mimeType); + + try { + await pipeline(fileResponse.stream, res); + } catch (error) { + this.logger.error('Front component stream failed mid-transfer', { + error, + }); + + if (!res.headersSent) { + throw new FrontComponentException( + 'Error streaming front component built file', + FrontComponentExceptionCode.FRONT_COMPONENT_NOT_READY, ); } - if (error instanceof FrontComponentException) { - throw error; - } - - throw new FrontComponentException( - `Error retrieving front component built file: ${error.message}`, - FrontComponentExceptionCode.FRONT_COMPONENT_NOT_READY, - ); + res.destroy(); } } } diff --git a/packages/twenty-server/src/engine/metadata-modules/front-component/front-component.service.ts b/packages/twenty-server/src/engine/metadata-modules/front-component/front-component.service.ts index 9961ae33ab..50c064d357 100644 --- a/packages/twenty-server/src/engine/metadata-modules/front-component/front-component.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/front-component/front-component.service.ts @@ -1,13 +1,14 @@ import { Injectable } from '@nestjs/common'; -import { Readable } from 'stream'; - import { FileFolder } from 'twenty-shared/types'; import { isDefined } from 'twenty-shared/utils'; import { ApplicationService } from 'src/engine/core-modules/application/application.service'; import { type FlatApplication } from 'src/engine/core-modules/application/types/flat-application.type'; import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service'; +import { type FileResponse } from 'src/engine/core-modules/file/types/file-response.type'; +import { getContentDisposition } from 'src/engine/core-modules/file/utils/get-content-disposition.utils'; +import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { WorkspaceManyOrAllFlatEntityMapsCacheService } from 'src/engine/metadata-modules/flat-entity/services/workspace-many-or-all-flat-entity-maps-cache.service'; import { findFlatEntityByIdInFlatEntityMapsOrThrow } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps-or-throw.util'; import { findFlatEntityByIdInFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps.util'; @@ -32,6 +33,7 @@ export class FrontComponentService { private readonly workspaceManyOrAllFlatEntityMapsCacheService: WorkspaceManyOrAllFlatEntityMapsCacheService, private readonly applicationService: ApplicationService, private readonly fileStorageService: FileStorageService, + private readonly twentyConfigService: TwentyConfigService, ) {} async findAll(workspaceId: string): Promise { @@ -296,13 +298,13 @@ export class FrontComponentService { return frontComponent; } - async getBuiltComponentStream({ + async getBuiltComponentPresignedUrlOrStream({ frontComponentId, workspaceId, }: { frontComponentId: string; workspaceId: string; - }): Promise { + }): Promise { const frontComponent = await this.findByIdOrThrow( frontComponentId, workspaceId, @@ -315,11 +317,29 @@ export class FrontComponentService { }, ); - return this.fileStorageService.readFile({ + const mimeType = 'application/javascript'; + const resourceIdentifier = { workspaceId, applicationUniversalIdentifier: application.universalIdentifier, fileFolder: FileFolder.BuiltFrontComponent, resourcePath: frontComponent.builtComponentPath, + }; + + const presignedUrl = await this.fileStorageService.getPresignedUrl({ + ...resourceIdentifier, + expiresInSeconds: this.twentyConfigService.get( + 'STORAGE_S3_PRESIGNED_URL_EXPIRES_IN', + ), + responseContentType: mimeType, + responseContentDisposition: getContentDisposition(mimeType), }); + + if (presignedUrl) { + return { type: 'redirect', presignedUrl }; + } + + const stream = await this.fileStorageService.readFile(resourceIdentifier); + + return { type: 'stream', stream, mimeType }; } } diff --git a/packages/twenty-server/test/integration/rest/suites/__snapshots__/front-component-built-js.integration-spec.ts.snap b/packages/twenty-server/test/integration/rest/suites/__snapshots__/front-component-built-js.integration-spec.ts.snap new file mode 100644 index 0000000000..1561dcddbb --- /dev/null +++ b/packages/twenty-server/test/integration/rest/suites/__snapshots__/front-component-built-js.integration-spec.ts.snap @@ -0,0 +1,29 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Front component built JS endpoint should return 404 for a non-existent front component ID 1`] = ` +{ + "body": { + "code": "FRONT_COMPONENT_NOT_FOUND", + "error": "Error", + "messages": [ + "Front component not found", + ], + "statusCode": 404, + }, + "status": 404, +} +`; + +exports[`Front component built JS endpoint should return 404 when front component exists but built file is missing on storage 1`] = ` +{ + "body": { + "code": "FRONT_COMPONENT_NOT_FOUND", + "error": "Error", + "messages": [ + "Front component built file not found", + ], + "statusCode": 404, + }, + "status": 404, +} +`; diff --git a/packages/twenty-server/test/integration/rest/suites/front-component-built-js.integration-spec.ts b/packages/twenty-server/test/integration/rest/suites/front-component-built-js.integration-spec.ts index ddc672b8e1..1d759b7443 100644 --- a/packages/twenty-server/test/integration/rest/suites/front-component-built-js.integration-spec.ts +++ b/packages/twenty-server/test/integration/rest/suites/front-component-built-js.integration-spec.ts @@ -2,6 +2,7 @@ import { createFrontComponent } from 'test/integration/metadata/suites/front-com import { deleteFrontComponent } from 'test/integration/metadata/suites/front-component/utils/delete-front-component.util'; import { seedBuiltFrontComponentFile } from 'test/integration/metadata/suites/front-component/utils/seed-built-front-component-file.util'; import { makeRestAPIRequest } from 'test/integration/rest/utils/make-rest-api-request.util'; +import { expectOneNotInternalServerErrorHttpResponseSnapshot } from 'test/integration/utils/expect-one-not-internal-server-error-http-response-snapshot.util'; const BUILT_COMPONENT_PATH = 'src/front-components/test-endpoint.mjs'; @@ -57,15 +58,51 @@ describe('Front component built JS endpoint', () => { it('should return 404 for a non-existent front component ID', async () => { const nonExistentId = '00000000-0000-0000-0000-000000000000'; - await makeRestAPIRequest({ + const response = await makeRestAPIRequest({ method: 'get', path: `/front-components/${nonExistentId}`, bearer: APPLE_JANE_ADMIN_ACCESS_TOKEN, - }) - .expect(404) - .expect((res) => { - expect(res.body.statusCode).toBe(404); + }); + + expectOneNotInternalServerErrorHttpResponseSnapshot(response); + }); + + it('should return 404 when front component exists but built file is missing on storage', async () => { + const missingBuiltPath = 'src/front-components/will-be-deleted.mjs'; + + const { cleanup } = await seedBuiltFrontComponentFile({ + builtComponentPath: missingBuiltPath, + }); + + const { data } = await createFrontComponent({ + expectToFail: false, + input: { + name: 'testMissingBuiltFile', + componentName: 'TestMissingBuiltFile', + sourceComponentPath: 'src/front-components/will-be-deleted.tsx', + builtComponentPath: missingBuiltPath, + builtComponentChecksum: 'will-be-deleted-checksum', + }, + }); + + const missingFileComponentId = data.createFrontComponent.id; + + cleanup(); + + try { + const response = await makeRestAPIRequest({ + method: 'get', + path: `/front-components/${missingFileComponentId}`, + bearer: APPLE_JANE_ADMIN_ACCESS_TOKEN, }); + + expectOneNotInternalServerErrorHttpResponseSnapshot(response); + } finally { + await deleteFrontComponent({ + expectToFail: false, + input: { id: missingFileComponentId }, + }); + } }); it('should return 403 when no token is provided', async () => {