🐛 [RUM-2721] fix upload retry

DataDog · Feb 7, 2024 · 3a612db · 3a612db
1 parent be0dec6
commit 3a612db
Show file tree

Hide file tree

Showing 11 changed files with 100 additions and 68 deletions.
diff --git a/src/commands/dsyms/interfaces.ts b/src/commands/dsyms/interfaces.ts
@@ -1,5 +1,3 @@
-import fs from 'fs'
-
 import {MultipartPayload, MultipartValue} from '../../helpers/upload'
 
 export interface Dsym {
@@ -24,7 +22,7 @@ export class CompressedDsym {
 
   public asMultipartPayload(): MultipartPayload {
     const content = new Map([
-      ['symbols_archive', {value: fs.createReadStream(this.archivePath), options: {filename: 'ios_symbols_archive'}}],
+      ['symbols_archive', {type: 'file', path: this.archivePath, options: {filename: 'ios_symbols_archive'}}],
       ['event', this.getMetadataPayload()],
     ])
 
@@ -37,6 +35,7 @@ export class CompressedDsym {
     const concatUUIDs = this.dsym.slices.map((slice) => slice.uuid).join()
 
     return {
+      type: 'string',
       options: {
         contentType: 'application/json',
         filename: 'event',

diff --git a/src/commands/flutter-symbols/__tests__/upload.test.ts b/src/commands/flutter-symbols/__tests__/upload.test.ts
@@ -1,11 +1,8 @@
-import {ReadStream} from 'fs'
 import os from 'os'
 
-import FormData from 'form-data'
-
 import {createCommand} from '../../../helpers/__tests__/fixtures'
 import {TrackedFilesMatcher, getRepositoryData} from '../../../helpers/git/format-git-sourcemaps-data'
-import {MultipartPayload} from '../../../helpers/upload'
+import {MultipartFileValue, MultipartPayload, MultipartStringValue} from '../../../helpers/upload'
 import {performSubCommand} from '../../../helpers/utils'
 import {version} from '../../../helpers/version'
 
@@ -359,12 +356,11 @@ describe('flutter-symbol upload', () => {
 
       expect(uploadMultipartHelper).toHaveBeenCalled()
       const payload = (uploadMultipartHelper as jest.Mock).mock.calls[0][1] as MultipartPayload
-      expect(JSON.parse(payload.content.get('event')?.value as string)).toStrictEqual(expectedMetadata)
-      const mappingFileItem = payload.content.get('jvm_mapping_file')
+      expect(JSON.parse((payload.content.get('event') as MultipartStringValue).value)).toStrictEqual(expectedMetadata)
+      const mappingFileItem = payload.content.get('jvm_mapping_file') as MultipartFileValue
       expect(mappingFileItem).toBeTruthy()
-      expect((mappingFileItem?.options as FormData.AppendOptions).filename).toBe('jvm_mapping')
-      expect(mappingFileItem?.value).toBeInstanceOf(ReadStream)
-      expect((mappingFileItem?.value as ReadStream).path).toBe(`${fixtureDir}/android/fake-mapping.txt`)
+      expect(mappingFileItem.options.filename).toBe('jvm_mapping')
+      expect(mappingFileItem.path).toBe(`${fixtureDir}/android/fake-mapping.txt`)
       expect(exitCode).toBe(0)
     })
 
@@ -412,11 +408,11 @@ describe('flutter-symbol upload', () => {
 
       expect(uploadMultipartHelper).toHaveBeenCalled()
       const payload = (uploadMultipartHelper as jest.Mock).mock.calls[0][1] as MultipartPayload
-      expect(JSON.parse(payload.content.get('event')?.value as string)).toStrictEqual(expectedMetadata)
-      const repoValue = payload.content.get('repository')
-      expect(JSON.parse(repoValue?.value as string)).toStrictEqual(expectedRepository)
-      expect((repoValue?.options as FormData.AppendOptions).filename).toBe('repository')
-      expect((repoValue?.options as FormData.AppendOptions).contentType).toBe('application/json')
+      expect(JSON.parse((payload.content.get('event') as MultipartStringValue).value)).toStrictEqual(expectedMetadata)
+      const repoValue = payload.content.get('repository') as MultipartStringValue
+      expect(JSON.parse(repoValue.value)).toStrictEqual(expectedRepository)
+      expect((repoValue?.options).filename).toBe('repository')
+      expect((repoValue?.options).contentType).toBe('application/json')
       expect(exitCode).toBe(0)
     })
 
@@ -663,19 +659,18 @@ describe('flutter-symbol upload', () => {
         const mockCalls = (uploadMultipartHelper as jest.Mock).mock.calls
         const index = mockCalls.findIndex((call) => {
           const checkPayload = call[1] as MultipartPayload
-          const eventPayload = checkPayload.content.get('event')?.value as string
+          const eventPayload = (checkPayload.content.get('event') as MultipartStringValue).value
 
           return eventPayload === JSON.stringify(expectedMetadata)
         })
         // Ensure the metadata matches at least one call
         expect(index).not.toBe(-1)
         const payload = mockCalls[index][1] as MultipartPayload
-        const mappingFileItem = payload.content.get('flutter_symbol_file')
+        const mappingFileItem = payload.content.get('flutter_symbol_file') as MultipartFileValue
         expect(mappingFileItem).toBeTruthy()
-        expect((mappingFileItem?.options as FormData.AppendOptions).filename).toBe('flutter_symbol_file')
-        expect(mappingFileItem?.value).toBeInstanceOf(ReadStream)
+        expect(mappingFileItem.options.filename).toBe('flutter_symbol_file')
         const expectedPath = `${fixtureDir}/dart-symbols/app.${expectedMetadata.platform}-${expectedMetadata.arch}.symbols`
-        expect((mappingFileItem?.value as ReadStream).path).toBe(expectedPath)
+        expect(mappingFileItem.path).toBe(expectedPath)
       })
 
       expect(exitCode).toBe(0)
@@ -725,17 +720,17 @@ describe('flutter-symbol upload', () => {
         const mockCalls = (uploadMultipartHelper as jest.Mock).mock.calls
         const index = mockCalls.findIndex((call) => {
           const checkPayload = call[1] as MultipartPayload
-          const eventPayload = checkPayload.content.get('event')?.value as string
+          const eventPayload = (checkPayload.content.get('event') as MultipartStringValue).value
 
           return eventPayload === JSON.stringify(expectedMetadata)
         })
         // Ensure the metadata matches at least one call
         expect(index).not.toBe(-1)
         const payload = mockCalls[index][1] as MultipartPayload
-        const repoValue = payload.content.get('repository')
-        expect(JSON.parse(repoValue?.value as string)).toStrictEqual(expectedRepository)
-        expect((repoValue?.options as FormData.AppendOptions).filename).toBe('repository')
-        expect((repoValue?.options as FormData.AppendOptions).contentType).toBe('application/json')
+        const repoValue = payload.content.get('repository') as MultipartStringValue
+        expect(JSON.parse(repoValue.value)).toStrictEqual(expectedRepository)
+        expect(repoValue.options.filename).toBe('repository')
+        expect(repoValue.options.contentType).toBe('application/json')
         expect(exitCode).toBe(0)
       })
 

diff --git a/src/commands/flutter-symbols/upload.ts b/src/commands/flutter-symbols/upload.ts
@@ -218,6 +218,7 @@ export class UploadCommand extends Command {
     }
 
     return {
+      type: 'string',
       options: {filename: 'repository', contentType: 'application/json'},
       value: JSON.stringify(repoPayload),
     }
@@ -318,10 +319,17 @@ export class UploadCommand extends Command {
 
     const payload = {
       content: new Map<string, MultipartValue>([
-        ['event', {value: JSON.stringify(metadata), options: {filename: 'event', contentType: 'application/json'}}],
+        [
+          'event',
+          {
+            type: 'string',
+            value: JSON.stringify(metadata),
+            options: {filename: 'event', contentType: 'application/json'},
+          },
+        ],
         [
           VALUE_NAME_JVM_MAPPING,
-          {value: fs.createReadStream(this.androidMappingLocation!), options: {filename: JVM_MAPPING_FILE_NAME}},
+          {type: 'file', path: this.androidMappingLocation!, options: {filename: JVM_MAPPING_FILE_NAME}},
         ],
       ]),
     }
@@ -384,10 +392,21 @@ export class UploadCommand extends Command {
         const metadata = this.getFlutterMetadata(fileMetadata.platform, fileMetadata.arch)
         const payload = {
           content: new Map<string, MultipartValue>([
-            ['event', {value: JSON.stringify(metadata), options: {filename: 'event', contentType: 'application/json'}}],
+            [
+              'event',
+              {
+                type: 'string',
+                value: JSON.stringify(metadata),
+                options: {filename: 'event', contentType: 'application/json'},
+              },
+            ],
             [
               VALUE_NAME_DART_MAPPING,
-              {value: fs.createReadStream(fileMetadata.filename), options: {filename: DART_SYMBOL_FILE_NAME}},
+              {
+                type: 'file',
+                path: fileMetadata.filename,
+                options: {filename: DART_SYMBOL_FILE_NAME},
+              },
             ],
           ]),
         }

diff --git a/src/commands/git-metadata/interfaces.ts b/src/commands/git-metadata/interfaces.ts
@@ -18,6 +18,7 @@ export class CommitInfo {
         [
           'repository',
           {
+            type: 'string',
             options: {
               contentType: 'application/json',
               filename: 'repository',
@@ -31,6 +32,7 @@ export class CommitInfo {
 
   private getMetadataPayload(cliVersion: string): MultipartValue {
     return {
+      type: 'string',
       options: {
         contentType: 'application/json',
         filename: 'event',

diff --git a/src/commands/react-native/__tests__/interfaces.test.ts b/src/commands/react-native/__tests__/interfaces.test.ts
@@ -1,4 +1,6 @@
-import fs from 'fs'
+import fs from 'fs/promises'
+
+import {MultipartFileValue} from '../../../helpers/upload'
 
 import {RNSourcemap} from '../interfaces'
 
@@ -15,14 +17,9 @@ describe('interfaces', () => {
       )
       sourcemap.removeSourcesContentFromSourceMap()
       const payload = sourcemap.asMultipartPayload('1.0', 'com.myapp', '1.2.3', '', 'android', '102030')
-      const sourcemapFileHandle = payload.content.get('source_map')?.value as fs.ReadStream
-
-      let fileContent = ''
-      sourcemapFileHandle.on('data', (chunk) => {
-        fileContent = `${fileContent}${chunk.toString()}`
-      })
+      const sourcemapFilePath = (payload.content.get('source_map') as MultipartFileValue).path
 
-      await new Promise((resolve) => sourcemapFileHandle.on('close', resolve))
+      const fileContent = await fs.readFile(sourcemapFilePath, 'utf8')
 
       expect(fileContent).toContain('"sources":["Users/me/datadog-ci/src/commands/sourcemaps/__tests__/git.test.ts"]')
       expect(fileContent).not.toContain('"sourcesContent"')

diff --git a/src/commands/react-native/interfaces.ts b/src/commands/react-native/interfaces.ts
@@ -26,10 +26,11 @@ export class RNSourcemap {
   ): MultipartPayload {
     const content = new Map<string, MultipartValue>([
       ['event', this.getMetadataPayload(cliVersion, service, version, projectPath, platform, build)],
-      ['source_map', {value: fs.createReadStream(this.sourcemapPath), options: {filename: 'source_map'}}],
+      ['source_map', {type: 'file', path: this.sourcemapPath, options: {filename: 'source_map'}}],
     ])
     if (this.gitData !== undefined && this.gitData.gitRepositoryPayload !== undefined) {
       content.set('repository', {
+        type: 'string',
         options: {
           contentType: 'application/json',
           filename: 'repository',
@@ -77,6 +78,7 @@ export class RNSourcemap {
     }
 
     return {
+      type: 'string',
       options: {
         contentType: 'application/json',
         filename: 'event',

diff --git a/src/commands/sourcemaps/interfaces.ts b/src/commands/sourcemaps/interfaces.ts
@@ -1,5 +1,3 @@
-import fs from 'fs'
-
 import {MultipartPayload, MultipartValue} from '../../helpers/upload'
 
 export class Sourcemap {
@@ -36,11 +34,12 @@ export class Sourcemap {
   ): MultipartPayload {
     const content = new Map<string, MultipartValue>([
       ['event', this.getMetadataPayload(cliVersion, service, version, projectPath)],
-      ['source_map', {value: fs.createReadStream(this.sourcemapPath), options: {filename: 'source_map'}}],
-      ['minified_file', {value: fs.createReadStream(this.minifiedFilePath), options: {filename: 'minified_file'}}],
+      ['source_map', {type: 'file', path: this.sourcemapPath, options: {filename: 'source_map'}}],
+      ['minified_file', {type: 'file', path: this.minifiedFilePath, options: {filename: 'minified_file'}}],
     ])
     if (this.gitData !== undefined && this.gitData.gitRepositoryPayload !== undefined) {
       content.set('repository', {
+        type: 'string',
         options: {
           contentType: 'application/json',
           filename: 'repository',
@@ -74,6 +73,7 @@ export class Sourcemap {
     }
 
     return {
+      type: 'string',
       options: {
         contentType: 'application/json',
         filename: 'event',

diff --git a/src/commands/unity-symbols/__tests__/upload.test.ts b/src/commands/unity-symbols/__tests__/upload.test.ts
@@ -1,11 +1,8 @@
-import {ReadStream} from 'fs'
 import os from 'os'
 
-import FormData from 'form-data'
-
 import {createCommand} from '../../../helpers/__tests__/fixtures'
 import {TrackedFilesMatcher, getRepositoryData} from '../../../helpers/git/format-git-sourcemaps-data'
-import {MultipartPayload} from '../../../helpers/upload'
+import {MultipartFileValue, MultipartPayload, MultipartStringValue} from '../../../helpers/upload'
 import {performSubCommand} from '../../../helpers/utils'
 import {version} from '../../../helpers/version'
 
@@ -207,11 +204,11 @@ describe('unity-symbols upload', () => {
 
       expect(uploadMultipartHelper).toHaveBeenCalled()
       const payload = (uploadMultipartHelper as jest.Mock).mock.calls[0][1] as MultipartPayload
-      expect(JSON.parse(payload.content.get('event')?.value as string)).toStrictEqual(expectedMetadata)
-      const repoValue = payload.content.get('repository')
-      expect(JSON.parse(repoValue?.value as string)).toStrictEqual(expectedRepository)
-      expect((repoValue?.options as FormData.AppendOptions).filename).toBe('repository')
-      expect((repoValue?.options as FormData.AppendOptions).contentType).toBe('application/json')
+      expect(JSON.parse((payload.content.get('event') as MultipartStringValue).value)).toStrictEqual(expectedMetadata)
+      const repoValue = payload.content.get('repository') as MultipartStringValue
+      expect(JSON.parse(repoValue.value)).toStrictEqual(expectedRepository)
+      expect((repoValue?.options).filename).toBe('repository')
+      expect((repoValue?.options).contentType).toBe('application/json')
       expect(exitCode).toBe(0)
     })
 
@@ -231,12 +228,11 @@ describe('unity-symbols upload', () => {
 
       expect(uploadMultipartHelper).toHaveBeenCalled()
       const payload = (uploadMultipartHelper as jest.Mock).mock.calls[0][1] as MultipartPayload
-      expect(JSON.parse(payload.content.get('event')?.value as string)).toStrictEqual(expectedMetadata)
-      const mappingFileItem = payload.content.get('il2cpp_mapping_file')
+      expect(JSON.parse((payload.content.get('event') as MultipartStringValue).value)).toStrictEqual(expectedMetadata)
+      const mappingFileItem = payload.content.get('il2cpp_mapping_file') as MultipartFileValue
       expect(mappingFileItem).toBeTruthy()
-      expect((mappingFileItem?.options as FormData.AppendOptions).filename).toBe('LineNumberMappings.json')
-      expect(mappingFileItem?.value).toBeInstanceOf(ReadStream)
-      expect((mappingFileItem?.value as ReadStream).path).toBe(`${fixtureDir}/LineNumberMappings.json`)
+      expect(mappingFileItem.options.filename).toBe('LineNumberMappings.json')
+      expect(mappingFileItem.path).toBe(`${fixtureDir}/LineNumberMappings.json`)
       expect(exitCode).toBe(0)
     })
 

diff --git a/src/commands/unity-symbols/upload.ts b/src/commands/unity-symbols/upload.ts
@@ -123,6 +123,7 @@ export class UploadCommand extends Command {
     }
 
     return {
+      type: 'string',
       options: {filename: 'repository', contentType: 'application/json'},
       value: JSON.stringify(repoPayload),
     }
@@ -215,10 +216,17 @@ export class UploadCommand extends Command {
 
     const payload = {
       content: new Map<string, MultipartValue>([
-        ['event', {value: JSON.stringify(metadata), options: {filename: 'event', contentType: 'application/json'}}],
+        [
+          'event',
+          {
+            type: 'string',
+            value: JSON.stringify(metadata),
+            options: {filename: 'event', contentType: 'application/json'},
+          },
+        ],
         [
           VALUE_NAME_IL2CPP_MAPPING,
-          {value: fs.createReadStream(il2cppMappingPath), options: {filename: IL2CPP_MAPPING_FILE_NAME}},
+          {type: 'file', path: il2cppMappingPath, options: {filename: IL2CPP_MAPPING_FILE_NAME}},
         ],
       ]),
     }

diff --git a/src/helpers/__tests__/upload.test.ts b/src/helpers/__tests__/upload.test.ts
@@ -101,9 +101,7 @@ describe('upload', () => {
 
       await upload(request)(
         {
-          content: new Map([
-            ['file', {value: fs.createReadStream(`${__dirname}/upload-fixtures/file.txt`), options: {}}],
-          ]),
+          content: new Map([['file', {type: 'file', path: `${__dirname}/upload-fixtures/file.txt`, options: {}}]]),
         },
         {
           onError: errorCallback,

diff --git a/src/helpers/upload.ts b/src/helpers/upload.ts
@@ -1,4 +1,4 @@
-import {ReadStream} from 'fs'
+import fs from 'fs'
 import {createGzip} from 'zlib'
 
 import FormData from 'form-data'
@@ -13,9 +13,18 @@ export interface MultipartPayload {
   content: Map<string, MultipartValue>
 }
 
-export interface MultipartValue {
-  options?: FormData.AppendOptions | string
-  value: string | ReadStream
+export type MultipartValue = MultipartStringValue | MultipartFileValue
+
+export interface MultipartStringValue {
+  type: 'string'
+  value: string
+  options: FormData.AppendOptions
+}
+
+export interface MultipartFileValue {
+  type: 'file'
+  path: string
+  options: FormData.AppendOptions
 }
 
 export interface UploadOptions {
@@ -92,7 +101,14 @@ const maxBodyLength = Infinity
 const uploadMultipart = async (request: RequestBuilder, payload: MultipartPayload, useGzip: boolean) => {
   const form = new FormData()
   payload.content.forEach((value: MultipartValue, key: string) => {
-    form.append(key, value.value, value.options)
+    switch (value.type) {
+      case 'string':
+        form.append(key, value.value, value.options)
+        break
+      case 'file':
+        form.append(key, fs.createReadStream(value.path), value.options)
+        break
+    }
   })
 
   let data: any = form