Skip to content

Commit 91c2f8f

Browse files
amortemousqueclaude
andcommitted
refactor(sourcemaps): consolidate debug ID handling into debugId module
- addDebugIdToPayloads sets each payload's debugId and gates the abort - fold missing-debug-ID skip into validatePayload as an InvalidPayload - move debug-ID messages into renderer; keep extractDebugId pure - add debugId.test.ts coverage and partial-debug-id fixture Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 8f7bda1 commit 91c2f8f

11 files changed

Lines changed: 122 additions & 50 deletions

File tree

Lines changed: 48 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,26 @@
11
import fs from 'fs'
22

3-
import type {CommandContext} from '@datadog/datadog-ci-base'
4-
5-
import {createMockContext} from '@datadog/datadog-ci-base/helpers/__tests__/testing-tools'
6-
7-
import {extractDebugId} from '../debugId'
3+
import {addDebugIdToPayloads, extractDebugId} from '../debugId'
4+
import {Sourcemap} from '../interfaces'
85

96
const DEBUG_ID = '2f1d7f52-4e1b-4f7c-8c0d-2f4a5f6d8e91'
107

11-
const mockFileContent = (content: string) => {
12-
jest.spyOn(fs, 'readFileSync').mockReturnValueOnce(content)
13-
}
8+
const makeSourcemap = (minifiedFilePath: string) =>
9+
new Sourcemap(minifiedFilePath, `https://static.com/${minifiedFilePath}`, `${minifiedFilePath}.map`, minifiedFilePath)
1410

15-
describe('extractDebugId', () => {
16-
let context: CommandContext
11+
// Mocks fs.readFileSync to return the given content keyed by minified file path.
12+
const mockFilesByPath = (contentByPath: Record<string, string>) => {
13+
jest.spyOn(fs, 'readFileSync').mockImplementation((path: unknown) => {
14+
const content = contentByPath[path as string]
15+
if (content === undefined) {
16+
throw new Error(`ENOENT: ${String(path)}`)
17+
}
1718

18-
beforeEach(() => {
19-
context = createMockContext() as CommandContext
19+
return content
2020
})
21+
}
2122

23+
describe('extractDebugId', () => {
2224
afterEach(() => {
2325
jest.restoreAllMocks()
2426
})
@@ -34,25 +36,49 @@ describe('extractDebugId', () => {
3436
[`var x=1;({"ddDebugId":"${DEBUG_ID}"});var y=2;`],
3537
[`var x=1;\n{"ddDebugId": "${DEBUG_ID}"}\nvar y=2;`],
3638
])('%s', (content: string) => {
37-
mockFileContent(content)
38-
expect(extractDebugId('bundle.js', context)).toBe(DEBUG_ID)
39-
expect(context.stderr.toString()).toBe('')
39+
jest.spyOn(fs, 'readFileSync').mockReturnValueOnce(content)
40+
expect(extractDebugId('bundle.js')).toBe(DEBUG_ID)
4041
})
4142
})
4243

4344
describe('missing or unreadable', () => {
44-
test('returns undefined and writes to stderr when snippet is absent', () => {
45-
mockFileContent('var x = 1; console.log("hello");')
46-
expect(extractDebugId('bundle.js', context)).toBeUndefined()
47-
expect(context.stderr.toString()).toContain('Debug ID not found')
45+
test('returns undefined when snippet is absent', () => {
46+
jest.spyOn(fs, 'readFileSync').mockReturnValueOnce('var x = 1; console.log("hello");')
47+
expect(extractDebugId('bundle.js')).toBeUndefined()
4848
})
4949

50-
test('returns undefined and writes to stderr when file cannot be read', () => {
50+
test('returns undefined when file cannot be read', () => {
5151
jest.spyOn(fs, 'readFileSync').mockImplementationOnce(() => {
5252
throw new Error('ENOENT: no such file or directory')
5353
})
54-
expect(extractDebugId('nonexistent.js', context)).toBeUndefined()
55-
expect(context.stderr.toString()).toContain('Cannot extract Debug ID')
54+
expect(extractDebugId('nonexistent.js')).toBeUndefined()
55+
})
56+
})
57+
})
58+
59+
describe('addDebugIdToPayloads', () => {
60+
afterEach(() => {
61+
jest.restoreAllMocks()
62+
})
63+
64+
test('stores each debug ID on its payload and returns true when any is found', () => {
65+
mockFilesByPath({
66+
'a.min.js': `{"ddDebugId":"${DEBUG_ID}"}`,
67+
'b.min.js': 'var x = 1;',
5668
})
69+
const withId = makeSourcemap('a.min.js')
70+
const withoutId = makeSourcemap('b.min.js')
71+
72+
expect(addDebugIdToPayloads([withId, withoutId])).toBe(true)
73+
expect(withId.debugId).toBe(DEBUG_ID)
74+
expect(withoutId.debugId).toBeUndefined()
75+
})
76+
77+
test('returns false when no payload has a debug ID', () => {
78+
mockFilesByPath({'a.min.js': 'var x = 1;', 'b.min.js': 'var y = 2;'})
79+
const payloads = [makeSourcemap('a.min.js'), makeSourcemap('b.min.js')]
80+
81+
expect(addDebugIdToPayloads(payloads)).toBe(false)
82+
expect(payloads.every((p) => p.debugId === undefined)).toBe(true)
5783
})
5884
})

packages/base/src/commands/sourcemaps/__tests__/fixtures/bundle-with-partial-debug-id/a.min.js

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/base/src/commands/sourcemaps/__tests__/fixtures/bundle-with-partial-debug-id/a.min.js.map

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/base/src/commands/sourcemaps/__tests__/fixtures/bundle-with-partial-debug-id/b.min.js

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/base/src/commands/sourcemaps/__tests__/fixtures/bundle-with-partial-debug-id/b.min.js.map

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/base/src/commands/sourcemaps/__tests__/upload.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,25 @@ describe('execute', () => {
301301
)
302302
})
303303

304+
test('debug id missing in all files aborts with exit 1', async () => {
305+
const {context, code} = await runCLIWithDebugId(['./src/commands/sourcemaps/__tests__/fixtures/basic'])
306+
expect(code).toBe(1)
307+
expect(context.stderr.toString()).toContain('No debug ID found in any minified file')
308+
expect(context.stdout.toString()).not.toContain('[DRYRUN] Uploading sourcemap')
309+
})
310+
311+
test('debug id missing in some files skips only those files', async () => {
312+
const {context, code} = await runCLIWithDebugId([
313+
'./src/commands/sourcemaps/__tests__/fixtures/bundle-with-partial-debug-id',
314+
])
315+
expect(code).toBe(0)
316+
const stdout = context.stdout.toString()
317+
expect(stdout).toContain('[DRYRUN] Uploading sourcemap')
318+
expect(stdout).toContain('a.min.js.map')
319+
expect(stdout).toContain('because no debug ID was found')
320+
expect(stdout).toContain('b.min.js.map')
321+
})
322+
304323
test('relative path with double dots', async () => {
305324
const {context, code} = await runCLI(['./src/commands/sourcemaps/__tests__/doesnotexist/../fixtures/basic'])
306325
const output = context.stdout.toString().split('\n')
Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,32 @@
11
import fs from 'fs'
22

3-
import type {CommandContext} from '@datadog/datadog-ci-base'
3+
import type {Sourcemap} from './interfaces'
44

55
const DD_DEBUG_ID_REGEX = /["']ddDebugId["']\s*:\s*["']([^"']+)["']/
66

7-
export const extractDebugId = (filePath: string, context: CommandContext): string | undefined => {
7+
export const extractDebugId = (filePath: string): string | undefined => {
88
try {
99
const source = fs.readFileSync(filePath, 'utf-8')
10-
const match = source.match(DD_DEBUG_ID_REGEX)
11-
if (match) {
12-
return match[1]
10+
11+
return source.match(DD_DEBUG_ID_REGEX)?.[1]
12+
} catch {
13+
// Unreadable file: treated as having no debug ID.
14+
return undefined
15+
}
16+
}
17+
18+
/**
19+
* Adds the debug ID extracted from each payload's minified file onto the
20+
* payload. Returns true if at least one payload has a debug ID.
21+
*/
22+
export const addDebugIdToPayloads = (payloads: Sourcemap[]): boolean => {
23+
let hasAnyDebugId = false
24+
for (const payload of payloads) {
25+
payload.debugId = extractDebugId(payload.minifiedFilePath)
26+
if (payload.debugId !== undefined) {
27+
hasAnyDebugId = true
1328
}
14-
context.stderr.write(`Debug ID not found in ${filePath}\n`)
15-
} catch (err) {
16-
const errorMsg = err instanceof Error ? err.message : String(err)
17-
context.stderr.write(`Cannot extract Debug ID from ${filePath}: ${errorMsg}\n`)
1829
}
1930

20-
return undefined
31+
return hasAnyDebugId
2132
}

packages/base/src/commands/sourcemaps/interfaces.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type {CommandContext} from '@datadog/datadog-ci-base'
22
import type {MultipartPayload, MultipartValue} from '@datadog/datadog-ci-base/helpers/upload'
33

44
export class Sourcemap {
5+
public debugId?: string
56
public gitData?: GitData
67
public minifiedFilePath: string
78
public minifiedPathPrefix?: string
@@ -49,21 +50,15 @@ export class Sourcemap {
4950
}
5051
}
5152

52-
private getMetadataPayload({
53-
cliVersion,
54-
service,
55-
version,
56-
projectPath,
57-
debugId,
58-
}: SourcemapUploadOptions): MultipartValue {
53+
private getMetadataPayload({cliVersion, service, version, projectPath}: SourcemapUploadOptions): MultipartValue {
5954
const metadata: {[k: string]: any} = {
6055
cli_version: cliVersion,
6156
project_path: projectPath,
6257
type: 'js_sourcemap',
6358
service,
6459
version,
6560
minified_url: this.minifiedUrl,
66-
debug_id: debugId,
61+
debug_id: this.debugId,
6762
}
6863

6964
if (this.gitData !== undefined) {
@@ -85,7 +80,6 @@ export class Sourcemap {
8580
export interface SourcemapUploadOptions {
8681
cliVersion: string
8782
context: CommandContext
88-
debugId?: string
8983
projectPath?: string
9084
service?: string
9185
version?: string

packages/base/src/commands/sourcemaps/renderer.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ export const renderFailedUpload = (sourcemap: Sourcemap, errorMessage: string) =
3838
return chalk.red(`${ICONS.FAILED} Failed upload sourcemap for ${sourcemapPathBold}: ${errorMessage}\n`)
3939
}
4040

41+
export const renderNoDebugIdFound = () => 'No debug ID found in any minified file. Aborting upload.\n'
42+
4143
export const renderRetriedUpload = (payload: Sourcemap, errorMessage: string, attempt: number) => {
4244
const sourcemapPathBold = `[${chalk.bold.dim(payload.sourcemapPath)}]`
4345

@@ -140,8 +142,8 @@ export const renderCommandInfo = (
140142
return fullStr
141143
}
142144

143-
export const renderUpload = (sourcemap: Sourcemap, debugId?: string): string => {
144-
const debugIdSuffix = debugId ? ` (debug ID: ${debugId})` : ''
145+
export const renderUpload = (sourcemap: Sourcemap): string => {
146+
const debugIdSuffix = sourcemap.debugId ? ` (debug ID: ${sourcemap.debugId})` : ''
145147

146148
return `Uploading sourcemap ${sourcemap.sourcemapPath} for JS file available at ${sourcemap.minifiedUrl}${debugIdSuffix}\n`
147149
}

packages/base/src/commands/sourcemaps/upload.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import {getRequestBuilder, buildPath} from '@datadog/datadog-ci-base/helpers/uti
3232
import * as validation from '@datadog/datadog-ci-base/helpers/validation'
3333
import {cliVersion} from '@datadog/datadog-ci-base/version'
3434

35-
import {extractDebugId} from './debugId'
35+
import {addDebugIdToPayloads} from './debugId'
3636
import {Sourcemap} from './interfaces'
3737
import {
3838
renderCommandInfo,
@@ -42,6 +42,7 @@ import {
4242
renderGitDataNotAttachedWarning,
4343
renderGitWarning,
4444
renderInvalidPrefix,
45+
renderNoDebugIdFound,
4546
renderRetriedUpload,
4647
renderSourcesNotFoundWarning,
4748
renderSuccessfulCommand,
@@ -138,6 +139,13 @@ export class SourcemapsUploadCommand extends BaseCommand {
138139
const useGit = this.disableGit === undefined || !this.disableGit
139140
const initialTime = Date.now()
140141
const payloads = await this.getPayloadsToUpload(useGit)
142+
143+
if (this.debugId && !addDebugIdToPayloads(payloads)) {
144+
this.context.stderr.write(renderNoDebugIdFound())
145+
146+
return 1
147+
}
148+
141149
const requestBuilder = this.getRequestBuilder()
142150
const uploadMultipart = this.upload(requestBuilder, metricsLogger, apiKeyValidator)
143151
try {
@@ -423,7 +431,7 @@ export class SourcemapsUploadCommand extends BaseCommand {
423431
): (sourcemap: Sourcemap) => Promise<UploadStatus> {
424432
return async (sourcemap: Sourcemap) => {
425433
try {
426-
validatePayload(sourcemap, this.context.stdout)
434+
validatePayload(sourcemap, this.context.stdout, this.debugId)
427435
} catch (error) {
428436
if (error instanceof InvalidPayload) {
429437
this.context.stdout.write(renderFailedUpload(sourcemap, error.message))
@@ -441,17 +449,15 @@ export class SourcemapsUploadCommand extends BaseCommand {
441449
return UploadStatus.Skipped
442450
}
443451

444-
const debugId = this.debugId ? extractDebugId(sourcemap.minifiedFilePath, this.context) : undefined
445452
const payload = sourcemap.asMultipartPayload({
446453
cliVersion: this.cliVersion,
447454
service: this.service,
448455
version: this.releaseVersion,
449456
projectPath: this.projectPath,
450-
debugId,
451457
context: this.context,
452458
})
453459
if (this.dryRun) {
454-
this.context.stdout.write(`[DRYRUN] ${renderUpload(sourcemap, debugId)}`)
460+
this.context.stdout.write(`[DRYRUN] ${renderUpload(sourcemap)}`)
455461

456462
return UploadStatus.Success
457463
}
@@ -470,7 +476,7 @@ export class SourcemapsUploadCommand extends BaseCommand {
470476
if (this.quiet) {
471477
return
472478
}
473-
this.context.stdout.write(renderUpload(sourcemap, debugId))
479+
this.context.stdout.write(renderUpload(sourcemap))
474480
},
475481
retries: 5,
476482
useGzip: true,

0 commit comments

Comments
 (0)