Skip to content

Commit b62dac6

Browse files
kvzclaude
andcommitted
fix: CLI exits with code 1 when jobs fail, fix AbortSignal listener leak
- Add hasFailures tracking to JobsPromise so CLI can detect failures - CLI assemblies create now returns exit code 1 when any job fails - Fix memory leak in awaitAssemblyCompletion: remove abort listener when sleep timeout resolves normally - Update CHANGELOG with better onPoll description 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent ce86458 commit b62dac6

File tree

6 files changed

+55
-16
lines changed

6 files changed

+55
-16
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ You may also want to refer to [GitHub releases](https://github.com/transloadit/n
55
## Unreleased
66

77
- Add `signal` option to `createAssembly()` for cancelling in-flight HTTP requests and TUS uploads via `AbortController`
8-
- Add `signal` and `onPoll` options to `awaitAssemblyCompletion()` for cancellation and custom polling control
8+
- Add `signal` and `onPoll` options to `awaitAssemblyCompletion()` for cancellation and early termination (useful for custom progress reporting or superseding assemblies in watch mode)
99
- Integrate transloadify CLI into the SDK, providing `assemblies`, `templates`, `bills`, and `assembly-notifications` commands
1010
- Add `--log-level (-l)` CLI option using syslog severity levels (err=3, warn=4, notice=5, info=6, debug=7, trace=8)
1111
- Apply stricter biome lint rules (noExplicitAny, useAwait, noForEach, noNonNullAssertion)

src/Transloadit.ts

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -422,17 +422,19 @@ export class Transloadit {
422422
throw new PollingTimeoutError('Polling timed out')
423423
}
424424

425-
// Make the sleep abortable
425+
// Make the sleep abortable, ensuring listener cleanup to prevent memory leaks
426426
await new Promise<void>((resolve, reject) => {
427-
const timeoutId = setTimeout(resolve, interval)
428-
signal?.addEventListener(
429-
'abort',
430-
() => {
431-
clearTimeout(timeoutId)
432-
reject(signal.reason ?? new DOMException('Aborted', 'AbortError'))
433-
},
434-
{ once: true },
435-
)
427+
const timeoutId = setTimeout(() => {
428+
signal?.removeEventListener('abort', onAbort)
429+
resolve()
430+
}, interval)
431+
432+
function onAbort() {
433+
clearTimeout(timeoutId)
434+
reject(signal?.reason ?? new DOMException('Aborted', 'AbortError'))
435+
}
436+
437+
signal?.addEventListener('abort', onAbort, { once: true })
436438
})
437439
}
438440
}

src/cli/JobsPromise.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
* Used to run multiple async operations in parallel while:
44
* 1. Reporting errors as they happen (via onError callback)
55
* 2. Waiting for all operations to complete at the end
6+
* 3. Tracking whether any failures occurred
67
*/
78
export default class JobsPromise {
89
private promises: Set<Promise<unknown>> = new Set()
910
private onError: ((err: unknown) => void) | null = null
11+
private _hasFailures = false
1012

1113
/**
1214
* Set the error handler for individual promise rejections.
@@ -29,13 +31,21 @@ export default class JobsPromise {
2931
const errorHandler = this.onError
3032
promise
3133
.catch((err: unknown) => {
34+
this._hasFailures = true
3235
errorHandler(err)
3336
})
3437
.finally(() => {
3538
this.promises.delete(promise)
3639
})
3740
}
3841

42+
/**
43+
* Returns true if any tracked promise has rejected.
44+
*/
45+
get hasFailures(): boolean {
46+
return this._hasFailures
47+
}
48+
3949
/**
4050
* Wait for all tracked promises to settle.
4151
* Returns array of fulfilled values (rejects are filtered out

src/cli/assemblies-create.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,7 @@ export default async function run(
615615
del,
616616
reprocessStale,
617617
}: AssembliesCreateOptions,
618-
): Promise<unknown[]> {
618+
): Promise<{ results: unknown[]; hasFailures: boolean }> {
619619
// Quick fix for https://github.com/transloadit/transloadify/issues/13
620620
// Only default to stdout when output is undefined (not provided), not when explicitly null
621621
let resolvedOutput = output
@@ -811,8 +811,9 @@ export default async function run(
811811
reject(err)
812812
})
813813

814-
emitter.on('end', () => {
815-
resolve(jobsPromise.allSettled())
814+
emitter.on('end', async () => {
815+
const results = await jobsPromise.allSettled()
816+
resolve({ results, hasFailures: jobsPromise.hasFailures })
816817
})
817818
})
818819
}

src/cli/commands/assemblies.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ export class AssembliesCreateCommand extends AuthenticatedCommand {
104104
fieldsMap[key] = value
105105
}
106106

107-
await assembliesCreate(this.output, this.client, {
107+
const { hasFailures } = await assembliesCreate(this.output, this.client, {
108108
steps: this.steps,
109109
template: this.template,
110110
fields: fieldsMap,
@@ -115,7 +115,7 @@ export class AssembliesCreateCommand extends AuthenticatedCommand {
115115
del: this.deleteAfterProcessing,
116116
reprocessStale: this.reprocessStale,
117117
})
118-
return undefined
118+
return hasFailures ? 1 : undefined
119119
}
120120
}
121121

test/unit/cli/JobsPromise.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,30 @@ describe('JobsPromise', () => {
4343
'JobsPromise: error handler must be set before adding promises',
4444
)
4545
})
46+
47+
it('should track hasFailures when promise rejects', async () => {
48+
const jobs = new JobsPromise()
49+
jobs.setErrorHandler(() => {})
50+
51+
expect(jobs.hasFailures).toBe(false)
52+
53+
jobs.add(Promise.resolve('ok'))
54+
jobs.add(Promise.reject(new Error('fail')))
55+
56+
await jobs.allSettled()
57+
58+
expect(jobs.hasFailures).toBe(true)
59+
})
60+
61+
it('should have hasFailures false when all succeed', async () => {
62+
const jobs = new JobsPromise()
63+
jobs.setErrorHandler(() => {})
64+
65+
jobs.add(Promise.resolve('a'))
66+
jobs.add(Promise.resolve('b'))
67+
68+
await jobs.allSettled()
69+
70+
expect(jobs.hasFailures).toBe(false)
71+
})
4672
})

0 commit comments

Comments
 (0)