Front component s3 redirect (#21116)
# Introduction Unload the server of the file stream when possible Also fix inconsistent pipeline exception management Needs to highly be QA, not sure how the cors will behave here
This commit is contained in:
+49
-27
@@ -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();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+25
-5
@@ -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<FrontComponentDTO[]> {
|
||||
@@ -296,13 +298,13 @@ export class FrontComponentService {
|
||||
return frontComponent;
|
||||
}
|
||||
|
||||
async getBuiltComponentStream({
|
||||
async getBuiltComponentPresignedUrlOrStream({
|
||||
frontComponentId,
|
||||
workspaceId,
|
||||
}: {
|
||||
frontComponentId: string;
|
||||
workspaceId: string;
|
||||
}): Promise<Readable> {
|
||||
}): Promise<FileResponse> {
|
||||
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 };
|
||||
}
|
||||
}
|
||||
|
||||
+29
@@ -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,
|
||||
}
|
||||
`;
|
||||
+42
-5
@@ -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 () => {
|
||||
|
||||
Reference in New Issue
Block a user