diff --git a/src/http.ts b/src/http.ts index 6873ec6d..8e20d861 100644 --- a/src/http.ts +++ b/src/http.ts @@ -39,13 +39,18 @@ function isSubsetErrorCode(code: any, messages: { [c: number]: string }): cod return false; } -function generateError(errorCode: number, messages: { [c: number]: string }, apiName: string): ResultError { +function generateError( + errorCode: number, + messages: { [c: number]: string }, + apiName: string, + error?: string, +): ResultError { if (!isSubsetErrorCode(errorCode, messages)) { throw { errorCode, messages, apiName }; } const statusCode = errorCode; - const statusText = messages[errorCode]; + const statusText = error ?? messages[errorCode]; return { type: 'error', @@ -379,6 +384,22 @@ export async function getAnalysis( return generateError(res.errorCode, GET_ANALYSIS_ERROR_MESSAGES, 'getAnalysis'); } +export type ReportErrorCodes = + | GenericErrorTypes + | ErrorCodes.unauthorizedUser + | ErrorCodes.unauthorizedBundleAccess + | ErrorCodes.badRequest + | ErrorCodes.notFound; + +const REPORT_ERROR_MESSAGES: { [P in ReportErrorCodes]: string } = { + ...GENERIC_ERROR_MESSAGES, + [ErrorCodes.unauthorizedUser]: DEFAULT_ERROR_MESSAGES[ErrorCodes.unauthorizedUser], + [ErrorCodes.unauthorizedBundleAccess]: DEFAULT_ERROR_MESSAGES[ErrorCodes.unauthorizedBundleAccess], + [ErrorCodes.notFound]: DEFAULT_ERROR_MESSAGES[ErrorCodes.notFound], + [ErrorCodes.badRequest]: DEFAULT_ERROR_MESSAGES[ErrorCodes.badRequest], + [ErrorCodes.serverError]: 'Getting report failed', +}; + export interface UploadReportOptions extends GetAnalysisOptions { report: ReportOptions; } @@ -394,7 +415,7 @@ export type UploadReportResponseDto = ReportResult | AnalysisFailedResponse | An export async function initReport( options: UploadReportOptions, -): Promise> { +): Promise> { const config: Payload = { headers: { ...commonHttpHeaders(options), @@ -420,12 +441,10 @@ export async function initReport( const res = await makeRequest(config); if (res.success) return { type: 'success', value: res.body }; - return generateError(res.errorCode, GET_ANALYSIS_ERROR_MESSAGES, 'initReport'); + return generateError(res.errorCode, REPORT_ERROR_MESSAGES, 'initReport'); } -export async function getReport( - options: GetReportOptions, -): Promise> { +export async function getReport(options: GetReportOptions): Promise> { const config: Payload = { headers: { ...commonHttpHeaders(options), @@ -436,7 +455,7 @@ export async function getReport( const res = await makeRequest(config); if (res.success) return { type: 'success', value: res.body }; - return generateError(res.errorCode, GET_ANALYSIS_ERROR_MESSAGES, 'getReport'); + return generateError(res.errorCode, REPORT_ERROR_MESSAGES, 'getReport', res.error?.message); } export function getVerifyCallbackUrl(authHost: string): string { diff --git a/src/needle.ts b/src/needle.ts index 0f6ac361..a306152a 100644 --- a/src/needle.ts +++ b/src/needle.ts @@ -107,6 +107,11 @@ export async function makeRequest( emitter.apiRequestLog(`Requested url --> ${url} | error --> ${err}`); } + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access + const errorMessage = response?.body?.error; + if (errorMessage) { + error = error ?? new Error(errorMessage); + } errorCode = errorCode ?? ErrorCodes.serviceUnavailable; // Try to avoid breaking requests due to temporary network errors diff --git a/src/report.ts b/src/report.ts index 4265b732..ec7eb49d 100644 --- a/src/report.ts +++ b/src/report.ts @@ -18,11 +18,17 @@ const sleep = (duration: number) => new Promise(resolve => setTimeout(resolve, d async function pollReport( options: UploadReportOptions, ): Promise> { - // Return early if project name is not provided. const projectName = options.report?.projectName?.trim(); + const projectNameMaxLength = 64; if (!projectName || projectName.length === 0) { throw new Error('"project-name" must be provided for "report"'); } + if (projectName.length > projectNameMaxLength) { + throw new Error(`"project-name" must not exceed ${projectNameMaxLength} characters`); + } + if (/[^A-Za-z0-9-_/]/g.test(projectName)) { + throw new Error(`"project-name" must not contain spaces or special characters except [/-_]`); + } emitter.analyseProgress({ status: AnalysisStatus.waiting, diff --git a/tests/report.spec.ts b/tests/report.spec.ts index 399b7489..9c71ab7d 100644 --- a/tests/report.spec.ts +++ b/tests/report.spec.ts @@ -2,11 +2,13 @@ import path from 'path'; import { baseURL, sessionToken, source } from './constants/base'; import { reportBundle } from '../src/report'; import * as http from '../src/http'; +import * as needle from '../src/needle'; import { sampleProjectPath, initReportReturn, getReportReturn } from './constants/sample'; const mockInitReport = jest.spyOn(http, 'initReport'); mockInitReport.mockReturnValue(Promise.resolve({ type: 'success', value: initReportReturn })); -jest.spyOn(http, 'getReport').mockReturnValue(Promise.resolve({ type: 'success', value: getReportReturn })); +const mockGetReport = jest.spyOn(http, 'getReport'); +mockGetReport.mockReturnValue(Promise.resolve({ type: 'success', value: getReportReturn })); describe('Functional test for report', () => { const paths: string[] = [path.join(sampleProjectPath)]; @@ -51,12 +53,76 @@ describe('Functional test for report', () => { projectName: undefined, }; - expect(async () => { - await reportBundle({ + await expect( + reportBundle({ bundleHash: 'dummy-bundle', ...baseConfig, report: reportConfig, - }); - }).rejects.toThrowError('"project-name" must be provided for "report"'); + }), + ).rejects.toHaveProperty('message', '"project-name" must be provided for "report"'); + }); + + it('should fail report if the given project name exceeds the maximum length', async () => { + const longProjectName = 'a'.repeat(65); + const reportConfig = { + enabled: true, + projectName: longProjectName, + }; + + await expect( + reportBundle({ + bundleHash: 'dummy-bundle', + ...baseConfig, + report: reportConfig, + }), + ).rejects.toHaveProperty('message', `"project-name" must not exceed 64 characters`); + }); + + it('should fail report if the given project name includes invalid characters', async () => { + const invalidProjectName = '*&^%$'; + const reportConfig = { + enabled: true, + projectName: invalidProjectName, + }; + + await expect( + reportBundle({ + bundleHash: 'dummy-bundle', + ...baseConfig, + report: reportConfig, + }), + ).rejects.toHaveProperty('message', `"project-name" must not contain spaces or special characters except [/-_]`); + }); + + it('getReport should return error with received error message rather than generic error message', async () => { + const statusCode = 400; + const expectedErrorMessage = 'Dummy received error message'; + + mockGetReport.mockRestore(); + jest + .spyOn(needle, 'makeRequest') + .mockReturnValue( + Promise.resolve({ success: false, errorCode: statusCode, error: new Error(expectedErrorMessage) }), + ); + + const reportConfig = { + enabled: true, + projectName: 'test-project', + targetName: 'test-target', + targetRef: 'test-ref', + remoteRepoUrl: 'https://github.com/owner/repo', + }; + + await expect( + reportBundle({ + bundleHash: 'dummy-bundle', + ...baseConfig, + report: reportConfig, + }), + ).rejects.toMatchObject({ + apiName: 'getReport', + statusCode: statusCode, + statusText: expectedErrorMessage, + }); }); });