Skip to content

Commit

Permalink
Remove form-data dependency
Browse files Browse the repository at this point in the history
  • Loading branch information
yoannmoinet committed Jun 27, 2024
1 parent 9271c7c commit a9ce05f
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 44 deletions.
Binary file not shown.
1 change: 0 additions & 1 deletion packages/plugins/rum/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
"@dd/core": "workspace:*",
"async-retry": "1.3.3",
"chalk": "2.3.1",
"form-data": "4.0.0",
"glob": "7.1.6",
"outdent": "0.8.0",
"p-queue": "6.6.2",
Expand Down
14 changes: 7 additions & 7 deletions packages/plugins/rum/src/sourcemaps/payload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,21 @@ type SourcemapValidity = {
repeatedPrefix: string;
};

interface AppendOptions {
contentType?: string;
filename?: string;
export interface LocalAppendOptions {
contentType: string;
filename: string;
}

interface MultipartStringValue {
type: 'string';
value: string;
options: AppendOptions;
options: LocalAppendOptions;
}

interface MultipartFileValue {
type: 'file';
path: string;
options: AppendOptions;
options: LocalAppendOptions;
}

export type MultipartValue = MultipartStringValue | MultipartFileValue;
Expand Down Expand Up @@ -143,15 +143,15 @@ export const getPayload = async (
{
type: 'file',
path: sourcemap.sourcemapFilePath,
options: { filename: 'source_map' },
options: { filename: 'source_map', contentType: 'application/json' },
},
],
[
'minified_file',
{
type: 'file',
path: sourcemap.minifiedFilePath,
options: { filename: 'minified_file' },
options: { filename: 'minified_file', contentType: 'application/javascript' },
},
],
]);
Expand Down
55 changes: 39 additions & 16 deletions packages/plugins/rum/src/sourcemaps/sender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,35 @@
import type { Logger } from '@dd/core/log';
import type { GlobalContext } from '@dd/core/types';
import retry from 'async-retry';
import FormData from 'form-data';
import { File } from 'buffer';
import fs from 'fs';
import PQueue from 'p-queue';
import { Readable } from 'stream';
import type { Gzip } from 'zlib';
import { createGzip } from 'zlib';

import type { RumSourcemapsOptionsWithDefaults, Sourcemap } from '../types';

import type { Metadata, Payload } from './payload';
import type { LocalAppendOptions, Metadata, Payload } from './payload';
import { getPayload } from './payload';

const errorCodesNoRetry = [400, 403, 413];
const nbRetries = 5;

type DataResponse = { data: Gzip; headers: Record<string, string> };

export const doRequest = async (
url: string,
// Need a function to get new streams for each retry.
getData: () => { data: Gzip; headers: Record<string, string> },
getData: () => Promise<DataResponse> | DataResponse,
onRetry?: (error: Error, attempt: number) => void,
) => {
return retry(
async (bail: (e: Error) => void, attempt: number) => {
let response: Response;
try {
const { data, headers } = getData();
const { data, headers } = await getData();

response = await fetch(url, {
method: 'POST',
body: data,
Expand Down Expand Up @@ -72,32 +76,51 @@ export const doRequest = async (
);
};

// From a path, returns a File to use with native FormData and fetch.
const getFile = async (path: string, options: LocalAppendOptions) => {
// @ts-expect-error openAsBlob is not in the NodeJS types until 19+
if (typeof fs.openAsBlob === 'function') {
// Support NodeJS 19+
// @ts-expect-error openAsBlob is not in the NodeJS types until 19+
const blob = await fs.openAsBlob(path, { type: options.contentType });
return new File([blob], options.filename);
} else {
// Support NodeJS 18-
const stream = Readable.toWeb(fs.createReadStream(path));
const blob = await new Response(stream, {
headers: { 'Content-Type': options.contentType },
}).blob();
const file = new File([blob], options.filename);
return file;
}
};

// Use a function to get new streams for each retry.
export const getData =
(payload: Payload, defaultHeaders: Record<string, string> = {}) =>
(): { data: Gzip; headers: Record<string, string> } => {
// Using form-data as a dependency isn't really required.
// But this implementation makes it mandatory for the gzip step.
// NodeJS' fetch SHOULD support everything else from the native FormData primitive.
// content.options are totally optional and should only be filename.
// form.getHeaders() is natively handled by fetch and FormData.
// TODO: Remove form-data dependency.
async (): Promise<DataResponse> => {
const form = new FormData();
const gz = createGzip();

for (const [key, content] of payload.content) {
const value =
content.type === 'file' ? fs.createReadStream(content.path) : content.value;
content.type === 'file'
? // eslint-disable-next-line no-await-in-loop
await getFile(content.path, content.options)
: new Blob([content.value], { type: content.options.contentType });

form.append(key, value, content.options);
form.append(key, value, content.options.filename);
}

// GZip data.
const data = form.pipe(gz);
// GZip data, we use a Request to serialize the data and transform it into a stream.
const req = new Request('fake://url', { method: 'POST', body: form });
const formStream = Readable.fromWeb(req.body!);
const data = formStream.pipe(gz);

const headers = {
'Content-Encoding': 'gzip',
...defaultHeaders,
...form.getHeaders(),
...Object.fromEntries(req.headers.entries()),
};

return { data, headers };
Expand Down
22 changes: 15 additions & 7 deletions packages/tests/src/plugins/rum/sourcemaps/sender.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('RUM Plugin Sourcemaps', () => {
afterEach(() => {
vol.reset();
});
test('It should return the data and headers', async () => {
test('It should return the correct data and headers', async () => {
// Emulate some fixtures.
vol.fromJSON(
{
Expand All @@ -67,28 +67,35 @@ describe('RUM Plugin Sourcemaps', () => {
{
type: 'file',
path: '/path/to/sourcemap.js.map',
options: { filename: 'source_map' },
options: { filename: 'source_map', contentType: 'application/json' },
},
],
[
'minified_file',
{
type: 'file',
path: '/path/to/minified.min.js',
options: { filename: 'minified_file' },
options: {
filename: 'minified_file',
contentType: 'application/javascript',
},
},
],
]),
errors: [],
warnings: [],
};

const { data, headers } = getData(payload)();
const { data, headers } = await getData(payload)();
const zippedData = await readFully(data);
const unzippedData = unzipSync(zippedData).toString('utf-8');
const dataLines = unzippedData.split(/[\r\n]/g).filter(Boolean);
const boundary = headers['content-type'].split('boundary=').pop()!.replace(/-/g, '');
const boundary = headers['content-type']
.split('boundary=')
.pop()!
.replace(/^(-)+/g, '');

expect(boundary).toBeTruthy();
expect(dataLines[0]).toMatch(boundary);
expect(dataLines[dataLines.length - 1]).toMatch(boundary);
});
Expand All @@ -106,7 +113,7 @@ describe('RUM Plugin Sourcemaps', () => {
// Emulate some fixtures.
vol.fromJSON(
{
'/path/to/minified.min.js': 'Some JS File',
'/path/to/minified.min.js': 'Some JS File with some content.',
'/path/to/sourcemap.js.map':
'{"version":3,"sources":["/path/to/minified.min.js"]}',
},
Expand Down Expand Up @@ -150,8 +157,9 @@ describe('RUM Plugin Sourcemaps', () => {
});

describe('doRequest', () => {
// TODO: Use a mock server here instead of mocking fetch.
const gz = createGzip();
const getDataMock = () => ({ data: gz, headers: {} });
const getDataMock = () => Promise.resolve({ data: gz, headers: {} });

test('It should do a request', async () => {
const response = await doRequest('https://example.com', getDataMock);
Expand Down
14 changes: 1 addition & 13 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1764,7 +1764,6 @@ __metadata:
"@types/async-retry": "npm:1.4.8"
async-retry: "npm:1.3.3"
chalk: "npm:2.3.1"
form-data: "npm:4.0.0"
glob: "npm:7.1.6"
outdent: "npm:0.8.0"
p-queue: "npm:6.6.2"
Expand Down Expand Up @@ -4805,7 +4804,7 @@ __metadata:
languageName: node
linkType: hard

"combined-stream@npm:^1.0.6, combined-stream@npm:^1.0.8, combined-stream@npm:~1.0.6":
"combined-stream@npm:^1.0.6, combined-stream@npm:~1.0.6":
version: 1.0.8
resolution: "combined-stream@npm:1.0.8"
dependencies:
Expand Down Expand Up @@ -6362,17 +6361,6 @@ __metadata:
languageName: node
linkType: hard

"form-data@npm:4.0.0":
version: 4.0.0
resolution: "form-data@npm:4.0.0"
dependencies:
asynckit: "npm:^0.4.0"
combined-stream: "npm:^1.0.8"
mime-types: "npm:^2.1.12"
checksum: 10/7264aa760a8cf09482816d8300f1b6e2423de1b02bba612a136857413fdc96d7178298ced106817655facc6b89036c6e12ae31c9eb5bdc16aabf502ae8a5d805
languageName: node
linkType: hard

"form-data@npm:~2.3.2":
version: 2.3.3
resolution: "form-data@npm:2.3.3"
Expand Down

0 comments on commit a9ce05f

Please sign in to comment.