Skip to content

Commit e109dd9

Browse files
committed
ci(bun): prevent partial-state success
A second Bun invocation is unsafe when the generated workspace state from the first failure remains in place.\n\nAllow one second workflow attempt for transient runner failures, and start both from an empty root node_modules tree.
1 parent 64e83ee commit e109dd9

5 files changed

Lines changed: 31 additions & 132 deletions

File tree

.github/actions/install/action.yml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ runs:
55
steps:
66
- uses: nick-fields/retry@ad984534de44a9489a53aefd81eb77f87c70dc60 # v4.0.0
77
with:
8-
max_attempts: 3
8+
max_attempts: 2
99
timeout_minutes: 5
1010
retry_wait_seconds: 30
11-
command: bun install --frozen-lockfile --linker=hoisted --network-concurrency 8
11+
shell: bash
12+
# A retry against Bun's partial install state can skip lifecycle scripts. Start every attempt clean.
13+
command: >-
14+
node -e "require('node:fs').rmSync('node_modules', { recursive: true, force: true })" &&
15+
bun install --frozen-lockfile --linker=hoisted --network-concurrency 8

scripts/helpers/retry.js

Lines changed: 0 additions & 46 deletions
This file was deleted.

scripts/helpers/retry.spec.js

Lines changed: 0 additions & 68 deletions
This file was deleted.

scripts/install_plugin_modules.js

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ const latests = require('../packages/dd-trace/test/plugins/versions/package.json
1717
const { isRelativeRequire } = require('../packages/datadog-instrumentations/src/helpers/shared-utils')
1818
const exec = require('./helpers/exec')
1919
const mapWithConcurrency = require('./helpers/concurrency')
20-
const retry = require('./helpers/retry')
2120
const requirePackageJsonPath = require.resolve('../packages/dd-trace/src/require-package-json')
2221
const requirePackageJson = require(requirePackageJsonPath)
2322

@@ -547,13 +546,6 @@ async function assertWorkspaces () {
547546
}, null, 2) + '\n')
548547
}
549548

550-
/**
551-
* Install the generated versions/ workspaces.
552-
*
553-
* Some workspaces download large prebuilt binaries at postinstall time (e.g. Electron pulls one archive per major
554-
* from GitHub's release CDN), which intermittently fail with 5xx gateway errors. Retry with backoff so a brief CDN
555-
* outage doesn't fail the whole job.
556-
*/
557549
function install () {
558550
try {
559551
// versions/bunfig.toml pins `linker = "isolated"`, which gives every sandbox
@@ -565,14 +557,10 @@ function install () {
565557
// (moleculer's runtime `require('bluebird')` fallback, etc.) are wired
566558
// through `externals.js` `dep: true, forced: true` so they land as a direct
567559
// dep of the consuming sandbox rather than as a sibling workspace.
568-
retry(() => exec('bun install --trust', { cwd: folder() }), {
569-
onRetry: (error, attempt, delayMs) => process.stderr.write(
570-
`bun install attempt ${attempt} failed, retrying in ${delayMs / 1000}s: ${error.message}\n`
571-
),
572-
})
560+
exec('bun install --trust', { cwd: folder() })
573561
} catch (error) {
574-
// A failure that outlasts the retries is most often an unresolvable version: a declared range spans a major
575-
// version that was never published. Point at the fix instead of leaving a bare bun error.
562+
// A failure is most often an unresolvable version: a declared range spans a major version that was never
563+
// published. Point at the fix instead of leaving a bare bun error.
576564
throw new Error(
577565
'bun failed to install the generated versions/ workspaces. If a plugin declares a version range that spans a ' +
578566
'major version that was never published (non-consecutive majors), add that package to ' +

scripts/test/install-plugin-modules.spec.js

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,21 @@ describe('scripts/install_plugin_modules.js', function () {
109109
assert.match(result.stderr, /Original error:/)
110110
})
111111

112+
it('does not retry a failed install against partial state', () => {
113+
const traceFile = path.join(wrapperDirectory, 'failed-install-trace.ndjson')
114+
const failureMarker = path.join(wrapperDirectory, 'failed-install')
115+
const result = spawnInstall('not-a-plugin', {
116+
DD_TEST_FAIL_BUN_INSTALL_ONCE_FILE: failureMarker,
117+
DD_TEST_PACKAGE_MANAGER_TRACE_FILE: traceFile,
118+
})
119+
120+
assert.strictEqual(result.status, 1)
121+
assert.deepStrictEqual(
122+
fs.readFileSync(traceFile, 'utf8').trim().split('\n').map(JSON.parse),
123+
[['bun', 'install', '--trust']]
124+
)
125+
})
126+
112127
it('rejects conflicting declarative overrides', () => {
113128
const preload = path.join(wrapperDirectory, 'conflicting-overrides.js')
114129
fs.writeFileSync(preload, `
@@ -331,13 +346,19 @@ function createPackageManagerWrappers () {
331346
'use strict'
332347
333348
const { spawnSync } = require('node:child_process')
334-
const { appendFileSync } = require('node:fs')
349+
const { appendFileSync, existsSync, writeFileSync } = require('node:fs')
335350
336351
const args = process.argv.slice(2)
337352
if (process.env.DD_TEST_PACKAGE_MANAGER_TRACE_FILE) {
338353
appendFileSync(process.env.DD_TEST_PACKAGE_MANAGER_TRACE_FILE, JSON.stringify(['bun', ...args]) + '\n')
339354
}
340355
if (process.env.DD_TEST_FAIL_BUN_INSTALL === 'true' && args[0] === 'install') process.exit(1)
356+
if (process.env.DD_TEST_FAIL_BUN_INSTALL_ONCE_FILE &&
357+
args[0] === 'install' &&
358+
!existsSync(process.env.DD_TEST_FAIL_BUN_INSTALL_ONCE_FILE)) {
359+
writeFileSync(process.env.DD_TEST_FAIL_BUN_INSTALL_ONCE_FILE, '')
360+
process.exit(1)
361+
}
341362
const result = spawnSync(${JSON.stringify(bunBinary)}, args, { stdio: 'inherit' })
342363
if (result.error) throw result.error
343364
process.exit(result.status ?? 1)

0 commit comments

Comments
 (0)