refactor: extract artifact upload process from lib/modes/record.js (#…

…29240)

* refactor record.js to extract upload logic into ts
- streamlines the main uploadArtifact fn
- extracts artifact specific logic to artifact classes
- fully defines types for upload processing and reporting

* tweak refactors so system tests produce same snapshots

* some todos, fixes exports/imports from api/index.ts

* fix api export so it can be imported by ts files

* cleans up types

* extracting artifact metadata from options logs to debug but does not throw if errors are encountered

* fix type imports in print-run

* fix debug formatting for artifacts

* fix reporting successful protocol uploads

* change inheritence to strategy

* rm empty file

* Update packages/server/lib/cloud/artifacts/upload_artifacts.ts

* makes protocolManager optional to uploadArtifacts, fixes conditional accessor in protocol fatal error report

* missed a potentially undef

* convert to frozen object / keyof instead of string composition for artifact kinds

---------

Co-authored-by: Ryan Manuel <ryanm@cypress.io>
Co-authored-by: Jennifer Shehane <jennifer@cypress.io>

packages/server/lib/cloud/api/index.ts

@@ -58,6 +58,7 @@ export interface CypressRequestOptions extends OptionsWithUrl {
   cacheable?: boolean
 }
 
+// TODO: migrate to fetch from @cypress/request
 const rp = request.defaults((params: CypressRequestOptions, callback) => {
   let resp
 
@@ -274,22 +275,23 @@ type CreateRunResponse = {
   } | undefined
 }
 
-type ArtifactMetadata = {
+export type ArtifactMetadata = {
   url: string
-  fileSize?: number
+  fileSize?: number | bigint
   uploadDuration?: number
   success: boolean
   error?: string
+  errorStack?: string
 }
 
-type ProtocolMetadata = ArtifactMetadata & {
+export type ProtocolMetadata = ArtifactMetadata & {
   specAccess?: {
-    size: bigint
-    offset: bigint
+    size: number
+    offset: number
   }
 }
 
-type UpdateInstanceArtifactsPayload = {
+export type UpdateInstanceArtifactsPayload = {
   screenshots: ArtifactMetadata[]
   video?: ArtifactMetadata
   protocol?: ProtocolMetadata
@@ -298,7 +300,7 @@ type UpdateInstanceArtifactsPayload = {
 type UpdateInstanceArtifactsOptions = {
   runId: string
   instanceId: string
-  timeout: number | undefined
+  timeout?: number
 }
 
 let preflightResult = {
@@ -307,7 +309,10 @@ let preflightResult = {
 
 let recordRoutes = apiRoutes
 
-module.exports = {
+// Potential todos: Refactor to named exports, refactor away from `this.` in exports,
+// move individual exports to their own files & convert this to barrelfile
+
+export default {
   rp,
 
   // For internal testing
@@ -400,8 +405,10 @@ module.exports = {
       let script
 
       try {
-        if (captureProtocolUrl || process.env.CYPRESS_LOCAL_PROTOCOL_PATH) {
-          script = await this.getCaptureProtocolScript(captureProtocolUrl || process.env.CYPRESS_LOCAL_PROTOCOL_PATH)
+        const protocolUrl = captureProtocolUrl || process.env.CYPRESS_LOCAL_PROTOCOL_PATH
+
+        if (protocolUrl) {
+          script = await this.getCaptureProtocolScript(protocolUrl)
         }
       } catch (e) {
         debugProtocol('Error downloading capture code', e)

packages/server/lib/cloud/artifacts/artifact.ts

@@ -0,0 +1,113 @@
+import Debug from 'debug'
+import { performance } from 'perf_hooks'
+
+const debug = Debug('cypress:server:cloud:artifact')
+
+const isAggregateError = (err: any): err is AggregateError => {
+  return !!err.errors
+}
+
+export const ArtifactKinds = Object.freeze({
+  VIDEO: 'video',
+  SCREENSHOTS: 'screenshots',
+  PROTOCOL: 'protocol',
+})
+
+type ArtifactKind = typeof ArtifactKinds[keyof typeof ArtifactKinds]
+
+export interface IArtifact {
+  reportKey: ArtifactKind
+  uploadUrl: string
+  filePath: string
+  fileSize: number | bigint
+  upload: () => Promise<ArtifactUploadResult>
+}
+
+export interface ArtifactUploadResult {
+  success: boolean
+  error?: Error | string
+  url: string
+  pathToFile: string
+  fileSize?: number | bigint
+  key: ArtifactKind
+  errorStack?: string
+  allErrors?: Error[]
+  specAccess?: {
+    offset: number
+    size: number
+  }
+  uploadDuration?: number
+}
+
+export type ArtifactUploadStrategy<T> = (filePath: string, uploadUrl: string, fileSize: number | bigint) => T
+
+export class Artifact<T extends ArtifactUploadStrategy<UploadResponse>, UploadResponse extends Promise<any> = Promise<{}>> {
+  constructor (
+    public reportKey: ArtifactKind,
+    public readonly filePath: string,
+    public readonly uploadUrl: string,
+    public readonly fileSize: number | bigint,
+    private uploadStrategy: T,
+  ) {
+  }
+
+  public async upload (): Promise<ArtifactUploadResult> {
+    const startTime = performance.now()
+
+    this.debug('upload starting')
+
+    try {
+      const response = await this.uploadStrategy(this.filePath, this.uploadUrl, this.fileSize)
+
+      this.debug('upload succeeded: %O', response)
+
+      return this.composeSuccessResult(response ?? {}, performance.now() - startTime)
+    } catch (e) {
+      this.debug('upload failed: %O', e)
+
+      return this.composeFailureResult(e, performance.now() - startTime)
+    }
+  }
+
+  private debug (formatter: string = '', ...args: (string | object | number)[]) {
+    if (!debug.enabled) return
+
+    debug(`%s: %s -> %s (%dB) ${formatter}`, this.reportKey, this.filePath, this.uploadUrl, this.fileSize, ...args)
+  }
+
+  private commonResultFields (): Pick<ArtifactUploadResult, 'url' | 'pathToFile' | 'fileSize' | 'key'> {
+    return {
+      key: this.reportKey,
+      url: this.uploadUrl,
+      pathToFile: this.filePath,
+      fileSize: this.fileSize,
+    }
+  }
+
+  protected composeSuccessResult<T extends Object = {}> (response: T, uploadDuration: number): ArtifactUploadResult {
+    return {
+      ...response,
+      ...this.commonResultFields(),
+      success: true,
+      uploadDuration,
+    }
+  }
+
+  protected composeFailureResult<T extends Error> (err: T, uploadDuration: number): ArtifactUploadResult {
+    const errorReport = isAggregateError(err) ? {
+      error: err.errors[err.errors.length - 1].message,
+      errorStack: err.errors[err.errors.length - 1].stack,
+      allErrors: err.errors,
+    } : {
+      error: err.message,
+      errorStack: err.stack,
+    }
+
+    return {
+      ...errorReport,
+      ...this.commonResultFields(),
+      success: false,
+      uploadDuration,
+    }
+  }
+}

packages/server/lib/cloud/artifacts/file_upload_strategy.ts

@@ -0,0 +1,6 @@
+import { sendFile } from '../upload/send_file'
+import type { ArtifactUploadStrategy } from './artifact'
+
+export const fileUploadStrategy: ArtifactUploadStrategy<Promise<any>> = (filePath, uploadUrl) => {
+  return sendFile(filePath, uploadUrl)
+}

packages/server/lib/cloud/artifacts/protocol_artifact.ts

@@ -0,0 +1,61 @@
+import fs from 'fs/promises'
+import type { ProtocolManager } from '../protocol'
+import { IArtifact, ArtifactUploadStrategy, ArtifactUploadResult, Artifact, ArtifactKinds } from './artifact'
+
+interface ProtocolUploadStrategyResult {
+  success: boolean
+  fileSize: number | bigint
+  specAccess: {
+    offset: number
+    size: number
+  }
+}
+
+const createProtocolUploadStrategy = (protocolManager: ProtocolManager) => {
+  const strategy: ArtifactUploadStrategy<Promise<ProtocolUploadStrategyResult | {}>> =
+    async (filePath, uploadUrl, fileSize) => {
+      const fatalError = protocolManager.getFatalError()
+
+      if (fatalError) {
+        throw fatalError.error
+      }
+
+      const res = await protocolManager.uploadCaptureArtifact({ uploadUrl, fileSize, filePath })
+
+      return res ?? {}
+    }
+
+  return strategy
+}
+
+export const createProtocolArtifact = async (filePath: string, uploadUrl: string, protocolManager: ProtocolManager): Promise<IArtifact> => {
+  const { size } = await fs.stat(filePath)
+
+  return new Artifact('protocol', filePath, uploadUrl, size, createProtocolUploadStrategy(protocolManager))
+}
+
+export const composeProtocolErrorReportFromOptions = async ({
+  protocolManager,
+  protocolCaptureMeta,
+  captureUploadUrl,
+}: {
+  protocolManager?: ProtocolManager
+  protocolCaptureMeta: { url?: string, disabledMessage?: string }
+  captureUploadUrl?: string
+}): Promise<ArtifactUploadResult> => {
+  const url = captureUploadUrl || protocolCaptureMeta.url
+  const pathToFile = protocolManager?.getArchivePath()
+  const fileSize = pathToFile ? (await fs.stat(pathToFile))?.size : 0
+
+  const fatalError = protocolManager?.getFatalError()
+
+  return {
+    key: ArtifactKinds.PROTOCOL,
+    url: url ?? 'UNKNOWN',
+    pathToFile: pathToFile ?? 'UNKNOWN',
+    fileSize,
+    success: false,
+    error: fatalError?.error.message || 'UNKNOWN',
+    errorStack: fatalError?.error.stack || 'UNKNOWN',
+  }
+}

packages/server/lib/cloud/artifacts/screenshot_artifact.ts

@@ -0,0 +1,44 @@
+import fs from 'fs/promises'
+import Debug from 'debug'
+import { Artifact, IArtifact, ArtifactKinds } from './artifact'
+import { fileUploadStrategy } from './file_upload_strategy'
+
+const debug = Debug('cypress:server:cloud:artifacts:screenshot')
+
+const createScreenshotArtifact = async (filePath: string, uploadUrl: string): Promise<IArtifact | undefined> => {
+  try {
+    const { size } = await fs.stat(filePath)
+
+    return new Artifact(ArtifactKinds.SCREENSHOTS, filePath, uploadUrl, size, fileUploadStrategy)
+  } catch (e) {
+    debug('Error creating screenshot artifact: %O', e)
+
+    return
+  }
+}
+
+export const createScreenshotArtifactBatch = (
+  screenshotUploadUrls: {screenshotId: string, uploadUrl: string}[],
+  screenshotFiles: {screenshotId: string, path: string}[],
+): Promise<IArtifact[]> => {
+  const correlatedPaths = screenshotUploadUrls.map(({ screenshotId, uploadUrl }) => {
+    const correlatedFilePath = screenshotFiles.find((pathPair) => {
+      return pathPair.screenshotId === screenshotId
+    })?.path
+
+    return correlatedFilePath ? {
+      filePath: correlatedFilePath,
+      uploadUrl,
+    } : undefined
+  }).filter((pair): pair is { filePath: string, uploadUrl: string } => {
+    return !!pair
+  })
+
+  return Promise.all(correlatedPaths.map(({ filePath, uploadUrl }) => {
+    return createScreenshotArtifact(filePath, uploadUrl)
+  })).then((artifacts) => {
+    return artifacts.filter((artifact): artifact is IArtifact => {
+      return !!artifact
+    })
+  })
+}