From 912e59930aeaea36e82601c6ff0902638e9ea750 Mon Sep 17 00:00:00 2001 From: Eric Black Date: Fri, 22 May 2026 10:26:46 -0700 Subject: [PATCH 1/7] refactor: migrate LogDisplayer to @heroku/sdk streamLogs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the EventSource-based polling + manual session-recreate loop with a single 'for await' over the SDK's logSession.streamLogs async iterable. The SDK now owns: - generation detection (Cedar vs Fir) and the per-generation log session option shape - forcing tail=true on Fir, since the platform doesn't support a bounded session there - opening the logplex_url and parsing the chunked text/plain stream (the platform never spoke text/event-stream — EventSource was the wrong abstraction for this endpoint) - transparent session recreate after the platform's idle timeout when tailing The CLI side is now just transport plumbing (User-Agent + proxy via a custom fetch override), an AbortController hooked to SIGINT/SIGTERM, the EPIPE handler for piping into 'head'/'grep', and an onSessionCreated callback that prints the legacy 'Fetching logs...' notice for Fir on the first session. --- package-lock.json | 4 +- package.json | 2 +- src/lib/run/log-displayer.ts | 174 +++++++++++------------------------ 3 files changed, 58 insertions(+), 122 deletions(-) diff --git a/package-lock.json b/package-lock.json index a1901c98aa..966689ecb6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,7 +16,7 @@ "@heroku/heroku-cli-util": "^10.8.0", "@heroku/http-call": "^5.5.1", "@heroku/mcp-server": "^1.2.0", - "@heroku/sdk": "github:heroku/heroku-sdk#main", + "@heroku/sdk": "github:heroku/heroku-sdk#eb/feat/stream-logs", "@heroku/socksv5": "^0.0.9", "@inquirer/prompts": "^7.0", "@oclif/core": "^4.8.4", @@ -2668,7 +2668,7 @@ }, "node_modules/@heroku/sdk": { "version": "0.4.0", - "resolved": "git+ssh://git@github.com/heroku/heroku-sdk.git#efce1d4fffd1be97127f4fe78bc65b1e22d6d0de", + "resolved": "git+ssh://git@github.com/heroku/heroku-sdk.git#0382c5b83512aca9d1f595da2e327c3f7ef2757a", "license": "Apache-2.0", "dependencies": { "@heroku/heroku-fetch": "github:heroku/heroku-fetch", diff --git a/package.json b/package.json index 287808bca9..1e303625bf 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "@heroku/heroku-cli-util": "^10.8.0", "@heroku/http-call": "^5.5.1", "@heroku/mcp-server": "^1.2.0", - "@heroku/sdk": "github:heroku/heroku-sdk#main", + "@heroku/sdk": "github:heroku/heroku-sdk#eb/feat/stream-logs", "@heroku/socksv5": "^0.0.9", "@inquirer/prompts": "^7.0", "@oclif/core": "^4.8.4", diff --git a/src/lib/run/log-displayer.ts b/src/lib/run/log-displayer.ts index a0dc128c34..6d49db8573 100644 --- a/src/lib/run/log-displayer.ts +++ b/src/lib/run/log-displayer.ts @@ -1,11 +1,10 @@ import {APIClient} from '@heroku-cli/command' import * as color from '@heroku/heroku-cli-util/color' +import {HerokuSDK} from '@heroku/sdk' +import {logSessionExtensions} from '@heroku/sdk/extensions/platform' import {ux} from '@oclif/core/ux' -import {EventSource} from 'eventsource' import {HttpsProxyAgent} from 'https-proxy-agent' -import {getGenerationByAppId} from '../apps/generation.js' -import {LogSession} from '../types/fir.js' import colorize from './colorize.js' interface LogDisplayerOptions { @@ -18,141 +17,78 @@ interface LogDisplayerOptions { } export class LogDisplayer { - private heroku: APIClient - - constructor(heroku: APIClient) { - this.heroku = heroku - } - - public createEventSourceInstance(url: string, options?: any): EventSource { - return new EventSource(url, options) + // eslint-disable-next-line @typescript-eslint/no-unused-vars + constructor(private heroku: APIClient) { + // heroku is preserved on the constructor for back-compat with + // callers; the SDK manages its own auth via env / netrc. } async display(options: LogDisplayerOptions): Promise { this.setupProcessHandlers() - const firApp = (await this.getGenerationByAppId(options)) === 'fir' - const isTail = firApp || options.tail - - const requestBodyParameters = this.buildRequestBodyParameters(firApp, options) - - let recreateLogSession = false - do { - const logSession = await this.createLogSession(requestBodyParameters, options.app) - - try { - await this.readLogs( - logSession.logplex_url, - isTail, - firApp ? Number(process.env.HEROKU_LOG_STREAM_TIMEOUT || '15') * 60 * 1000 : undefined, - ) - } catch (error: unknown) { - const {message} = error as Error - if (message === 'Fir log stream timeout') - recreateLogSession = true - else - ux.error(message, {exit: 1}) - } - } while (recreateLogSession) - } + const controller = new AbortController() + const onAbort = () => controller.abort() + process.once('SIGINT', onAbort) + process.once('SIGTERM', onAbort) - private buildRequestBodyParameters(firApp: boolean, options: LogDisplayerOptions): Record { - const requestBodyParameters = { - source: options.source, - } + const sdk = new HerokuSDK({extensions: [logSessionExtensions]}) - if (firApp) { - process.stderr.write(color.info('Fetching logs...\n\n')) - Object.assign(requestBodyParameters, { + try { + for await (const line of sdk.platform.logSession.streamLogs(options.app, { dyno: options.dyno, - type: options.type, - }) - } else { - Object.assign(requestBodyParameters, { - dyno: options.dyno || options.type, + fetch: this.buildFetch(), lines: options.lines, + onSessionCreated({generation, isRecreate}) { + // Fir's stream takes a moment to provision; print a hint + // before the first session so users don't think we're + // hung. Don't repeat it on tail-timeout recreates. + if (generation === 'fir' && !isRecreate) { + process.stderr.write(color.info('Fetching logs...\n\n')) + } + }, + signal: controller.signal, + source: options.source, tail: options.tail, - }) + type: options.type, + })) { + ux.stdout(colorize(line)) + } + } catch (error) { + if (controller.signal.aborted) return + const message = error instanceof Error ? error.message : String(error) + ux.error(message, {exit: 1}) + } finally { + process.off('SIGINT', onAbort) + process.off('SIGTERM', onAbort) } - - return requestBodyParameters - } - - private async createLogSession(requestBodyParameters: Record, app: string): Promise { - const {body: logSession} = await this.heroku.post(`/apps/${app}/log-sessions`, { - body: requestBodyParameters, - headers: {Accept: 'application/vnd.heroku+json; version=3.sdk'}, - }) - return logSession } - private async getGenerationByAppId(options: LogDisplayerOptions): Promise { - const generation = await getGenerationByAppId(options.app, this.heroku) - return generation || '' - } + /** + * Custom fetch override that injects User-Agent and routes through + * an https proxy when one is configured. The SDK's streamLogs + * accepts a `fetch` option for exactly this kind of CLI/Node-only + * transport configuration. + */ + private buildFetch(): typeof fetch { + const userAgent = process.env.HEROKU_DEBUG_USER_AGENT || 'heroku-run' + const proxy = process.env.https_proxy || process.env.HTTPS_PROXY - private readLogs(logplexURL: string, isTail: boolean, recreateSessionTimeout?: number): Promise { - return new Promise((resolve, reject) => { - const userAgent = process.env.HEROKU_DEBUG_USER_AGENT || 'heroku-run' - const proxy = process.env.https_proxy || process.env.HTTPS_PROXY + return async (input, init) => { + const headers = new Headers(init?.headers) + headers.set('User-Agent', userAgent) - // Custom fetch function to handle headers and proxy // eslint-disable-next-line no-undef - const customFetch = async (input: RequestInfo | URL, init?: RequestInit) => { - const headers = new Headers(init?.headers) - headers.set('User-Agent', userAgent) - - // eslint-disable-next-line no-undef - const fetchOptions: RequestInit & {agent?: any} = { - ...init, - headers, - } - - // If proxy is set, use https-proxy-agent - if (proxy) { - const proxyAgent = new HttpsProxyAgent(proxy) - fetchOptions.agent = proxyAgent - } - - return fetch(input, fetchOptions) + const fetchOptions: RequestInit & {agent?: unknown} = { + ...init, + headers, } - const es = this.createEventSourceInstance(logplexURL, { - fetch: customFetch, - }) - - es.addEventListener('error', (err: Event) => { - // The new eventsource package provides message and code properties on errors - const errorEvent = err as any - if (errorEvent && (errorEvent.code || errorEvent.message)) { - const msg = (isTail && (errorEvent.code === 404 || errorEvent.code === 403)) - ? 'Log stream timed out. Please try again.' - : `Logs eventsource failed with: ${errorEvent.code}${errorEvent.message ? ` ${errorEvent.message}` : ''}` - reject(new Error(msg)) - es.close() - } - - if (!isTail) { - resolve() - es.close() - } - - // should only land here if --tail and no error status or message - }) - - es.addEventListener('message', (e: MessageEvent) => { - e.data.trim().split(/\n+/).forEach((line: string) => { - ux.stdout(colorize(line)) - }) - }) - - if (isTail && recreateSessionTimeout) { - setTimeout(() => { - reject(new Error('Fir log stream timeout')) - es.close() - }, recreateSessionTimeout) + if (proxy) { + fetchOptions.agent = new HttpsProxyAgent(proxy) } - }) + + return fetch(input, fetchOptions) + } } private setupProcessHandlers(): void { From 19519f23d039ec26ddd63775d4b0939491a5c792 Mon Sep 17 00:00:00 2001 From: Eric Black Date: Fri, 22 May 2026 10:49:36 -0700 Subject: [PATCH 2/7] refactor: drop CLI buildFetch wrapper; SDK now provides Node-aware default The SDK's streamLogs now defaults to a Node-aware fetch that adds a User-Agent and routes through undici.EnvHttpProxyAgent (which honors HTTP_PROXY / HTTPS_PROXY / NO_PROXY env vars). The CLI's local buildFetch wrapper is redundant and is removed. This also fixes a likely-broken proxy path: the previous CLI code passed an https-proxy-agent via the 'agent' option, which Node's native fetch ignores entirely. Proxy support has probably not worked since the CLI moved off node-fetch. The undici dispatcher in the SDK is the modern correct way to wire proxy support into native fetch. --- package-lock.json | 2 +- src/lib/run/log-displayer.ts | 30 ------------------------------ 2 files changed, 1 insertion(+), 31 deletions(-) diff --git a/package-lock.json b/package-lock.json index 966689ecb6..0cfaaadd52 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2668,7 +2668,7 @@ }, "node_modules/@heroku/sdk": { "version": "0.4.0", - "resolved": "git+ssh://git@github.com/heroku/heroku-sdk.git#0382c5b83512aca9d1f595da2e327c3f7ef2757a", + "resolved": "git+ssh://git@github.com/heroku/heroku-sdk.git#df73704894d2817ecfc4ad8a93024c5d1b1253cc", "license": "Apache-2.0", "dependencies": { "@heroku/heroku-fetch": "github:heroku/heroku-fetch", diff --git a/src/lib/run/log-displayer.ts b/src/lib/run/log-displayer.ts index 6d49db8573..4d2ef347fb 100644 --- a/src/lib/run/log-displayer.ts +++ b/src/lib/run/log-displayer.ts @@ -3,7 +3,6 @@ import * as color from '@heroku/heroku-cli-util/color' import {HerokuSDK} from '@heroku/sdk' import {logSessionExtensions} from '@heroku/sdk/extensions/platform' import {ux} from '@oclif/core/ux' -import {HttpsProxyAgent} from 'https-proxy-agent' import colorize from './colorize.js' @@ -36,7 +35,6 @@ export class LogDisplayer { try { for await (const line of sdk.platform.logSession.streamLogs(options.app, { dyno: options.dyno, - fetch: this.buildFetch(), lines: options.lines, onSessionCreated({generation, isRecreate}) { // Fir's stream takes a moment to provision; print a hint @@ -63,34 +61,6 @@ export class LogDisplayer { } } - /** - * Custom fetch override that injects User-Agent and routes through - * an https proxy when one is configured. The SDK's streamLogs - * accepts a `fetch` option for exactly this kind of CLI/Node-only - * transport configuration. - */ - private buildFetch(): typeof fetch { - const userAgent = process.env.HEROKU_DEBUG_USER_AGENT || 'heroku-run' - const proxy = process.env.https_proxy || process.env.HTTPS_PROXY - - return async (input, init) => { - const headers = new Headers(init?.headers) - headers.set('User-Agent', userAgent) - - // eslint-disable-next-line no-undef - const fetchOptions: RequestInit & {agent?: unknown} = { - ...init, - headers, - } - - if (proxy) { - fetchOptions.agent = new HttpsProxyAgent(proxy) - } - - return fetch(input, fetchOptions) - } - } - private setupProcessHandlers(): void { process.stdout.on('error', err => { if (err.code === 'EPIPE') { From 0c4b2bbb959108fc2817fcc4f6b0b6c8a40382a1 Mon Sep 17 00:00:00 2001 From: Eric Black Date: Fri, 22 May 2026 11:34:28 -0700 Subject: [PATCH 3/7] chore: bump @heroku/sdk pin to pick up HerokuApiClient-based streamLogs The SDK now routes the logplex fetch through HerokuApiClient with service: 'custom', which inherits proxy + UA support from heroku-fetch. The CLI's wrapper was already removed; this just picks up the new SHA where the SDK's internals match. --- package-lock.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 0cfaaadd52..c178abfeee 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2668,10 +2668,10 @@ }, "node_modules/@heroku/sdk": { "version": "0.4.0", - "resolved": "git+ssh://git@github.com/heroku/heroku-sdk.git#df73704894d2817ecfc4ad8a93024c5d1b1253cc", + "resolved": "git+ssh://git@github.com/heroku/heroku-sdk.git#a369965cec1d4f374802e5da52c721deec1cb945", "license": "Apache-2.0", "dependencies": { - "@heroku/heroku-fetch": "github:heroku/heroku-fetch", + "@heroku/heroku-fetch": "github:heroku/heroku-fetch#eb/feat/proxy-support", "@heroku/types": "github:heroku/heroku-types", "debug": "^4.4.0" }, From fc479c1e91046387bbeaf5ce9cb274415706d229 Mon Sep 17 00:00:00 2001 From: Eric Black Date: Fri, 22 May 2026 11:48:24 -0700 Subject: [PATCH 4/7] refactor: replace LogDisplayer class with displayLogs function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The LogDisplayer class wrapped a single public method (display) and required an unused APIClient constructor parameter (kept for back- compat after the SDK migration). With the SDK now owning generation detection, transport, and session recreate, the class adds nothing — fold it into a plain async function. Both call sites (logs and run/detached) keep a static reference (Cmd.displayLogs = displayLogs) so existing sinon stubs can swap it out for tests, mirroring the pattern used by pipelines:promote's Promote.promotePipeline. (Drop the explanatory comment on those static references — the pattern's now repeated enough to be obvious.) Replaces the 412-line log-displayer.unit.test.ts (which was tightly coupled to the old EventSource + getGenerationByAppId architecture) with a small unit test that asserts on the streamLogs call arguments. Real network behavior is covered by the existing integration test at test/integration/logs.integration.test.ts. --- src/commands/logs.ts | 9 +- src/commands/pipelines/promote.ts | 2 - src/commands/run/detached.ts | 6 +- src/lib/run/log-displayer.ts | 101 ++--- test/unit/commands/logs.unit.test.ts | 4 +- test/unit/lib/run/log-displayer.unit.test.ts | 446 +++---------------- 6 files changed, 105 insertions(+), 463 deletions(-) diff --git a/src/commands/logs.ts b/src/commands/logs.ts index 1778d92b46..facafcfd6e 100644 --- a/src/commands/logs.ts +++ b/src/commands/logs.ts @@ -4,7 +4,7 @@ import * as color from '@heroku/heroku-cli-util/color' import {ux} from '@oclif/core/ux' import tsheredoc from 'tsheredoc' -import {LogDisplayer} from '../lib/run/log-displayer.js' +import {displayLogs} from '../lib/run/log-displayer.js' const heredoc = tsheredoc.default @@ -13,6 +13,7 @@ export default class Logs extends Command { display recent log output disable colors with --no-color, HEROKU_LOGS_COLOR=0, or HEROKU_COLOR=0 ` + public static displayLogs = displayLogs static examples = [ `${color.command('heroku logs --app=my-app')}`, `${color.command('heroku logs --num=50 --app=my-app')}`, @@ -79,15 +80,13 @@ export default class Logs extends Command { if (forceColors) ux.warn('The --force-colors flag is deprecated. Use FORCE_COLORS=true to force colors.') - const options = { + await Logs.displayLogs({ app, dyno, lines: num || 100, source, tail, type: type || ps, - } - const displayer = new LogDisplayer(this.heroku) - await displayer.display(options) + }) } } diff --git a/src/commands/pipelines/promote.ts b/src/commands/pipelines/promote.ts index 7c9c8a03d0..99c7ac8b13 100644 --- a/src/commands/pipelines/promote.ts +++ b/src/commands/pipelines/promote.ts @@ -39,8 +39,6 @@ export default class Promote extends Command { description: 'comma separated list of apps to promote to', }), } - // Static reference so tests can stub the SDK call without changing the - // command's behavior in production. public static promotePipeline = promotePipeline async run() { diff --git a/src/commands/run/detached.ts b/src/commands/run/detached.ts index 3971bac520..b1ad5b29bc 100644 --- a/src/commands/run/detached.ts +++ b/src/commands/run/detached.ts @@ -5,10 +5,11 @@ import {ux} from '@oclif/core/ux' import Dyno from '../../lib/run/dyno.js' import {buildCommandWithLauncher} from '../../lib/run/helpers.js' -import {LogDisplayer} from '../../lib/run/log-displayer.js' +import {displayLogs} from '../../lib/run/log-displayer.js' export default class RunDetached extends Command { static description = 'run a detached dyno, where output is sent to your logs' + public static displayLogs = displayLogs static examples = [ color.command('heroku run:detached ls'), ] @@ -48,8 +49,7 @@ export default class RunDetached extends Command { await dyno.start() if (flags.tail) { - const displayer = new LogDisplayer(this.heroku) - await displayer.display({ + await RunDetached.displayLogs({ app: flags.app, dyno: dyno.dyno?.name, tail: true, diff --git a/src/lib/run/log-displayer.ts b/src/lib/run/log-displayer.ts index 4d2ef347fb..43f5d38b04 100644 --- a/src/lib/run/log-displayer.ts +++ b/src/lib/run/log-displayer.ts @@ -1,4 +1,3 @@ -import {APIClient} from '@heroku-cli/command' import * as color from '@heroku/heroku-cli-util/color' import {HerokuSDK} from '@heroku/sdk' import {logSessionExtensions} from '@heroku/sdk/extensions/platform' @@ -6,8 +5,8 @@ import {ux} from '@oclif/core/ux' import colorize from './colorize.js' -interface LogDisplayerOptions { - app: string, +export interface LogDisplayerOptions { + app: string dyno?: string lines?: number source?: string @@ -15,66 +14,52 @@ interface LogDisplayerOptions { type?: string } -export class LogDisplayer { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - constructor(private heroku: APIClient) { - // heroku is preserved on the constructor for back-compat with - // callers; the SDK manages its own auth via env / netrc. - } - - async display(options: LogDisplayerOptions): Promise { - this.setupProcessHandlers() +export async function displayLogs(options: LogDisplayerOptions): Promise { + setupProcessHandlers() - const controller = new AbortController() - const onAbort = () => controller.abort() - process.once('SIGINT', onAbort) - process.once('SIGTERM', onAbort) + const controller = new AbortController() + const onAbort = () => controller.abort() + process.once('SIGINT', onAbort) + process.once('SIGTERM', onAbort) - const sdk = new HerokuSDK({extensions: [logSessionExtensions]}) + const {platform} = new HerokuSDK({extensions: [logSessionExtensions]}) - try { - for await (const line of sdk.platform.logSession.streamLogs(options.app, { - dyno: options.dyno, - lines: options.lines, - onSessionCreated({generation, isRecreate}) { - // Fir's stream takes a moment to provision; print a hint - // before the first session so users don't think we're - // hung. Don't repeat it on tail-timeout recreates. - if (generation === 'fir' && !isRecreate) { - process.stderr.write(color.info('Fetching logs...\n\n')) - } - }, - signal: controller.signal, - source: options.source, - tail: options.tail, - type: options.type, - })) { - ux.stdout(colorize(line)) - } - } catch (error) { - if (controller.signal.aborted) return - const message = error instanceof Error ? error.message : String(error) - ux.error(message, {exit: 1}) - } finally { - process.off('SIGINT', onAbort) - process.off('SIGTERM', onAbort) + try { + for await (const line of platform.logSession.streamLogs(options.app, { + dyno: options.dyno, + lines: options.lines, + onSessionCreated({generation, isRecreate}) { + // Fir's stream takes a moment to provision; print a hint + // before the first session so users don't think we're + // hung. Don't repeat it on tail-timeout recreates. + if (generation === 'fir' && !isRecreate) { + process.stderr.write(color.info('Fetching logs...\n\n')) + } + }, + signal: controller.signal, + source: options.source, + tail: options.tail, + type: options.type, + })) { + ux.stdout(colorize(line)) } - } - - private setupProcessHandlers(): void { - process.stdout.on('error', err => { - if (err.code === 'EPIPE') { - // eslint-disable-next-line n/no-process-exit, unicorn/no-process-exit - process.exit(0) - } else { - ux.error(err.stack, {exit: 1}) - } - }) + } catch (error) { + if (controller.signal.aborted) return + const message = error instanceof Error ? error.message : String(error) + ux.error(message, {exit: 1}) + } finally { + process.off('SIGINT', onAbort) + process.off('SIGTERM', onAbort) } } -// Default export for backward compatibility -export default async function logDisplayer(heroku: APIClient, options: LogDisplayerOptions): Promise { - const displayer = new LogDisplayer(heroku) - await displayer.display(options) +function setupProcessHandlers(): void { + process.stdout.on('error', err => { + if (err.code === 'EPIPE') { + // eslint-disable-next-line n/no-process-exit, unicorn/no-process-exit + process.exit(0) + } else { + ux.error(err.stack, {exit: 1}) + } + }) } diff --git a/test/unit/commands/logs.unit.test.ts b/test/unit/commands/logs.unit.test.ts index 3280d64b80..83ba8828b6 100644 --- a/test/unit/commands/logs.unit.test.ts +++ b/test/unit/commands/logs.unit.test.ts @@ -4,14 +4,12 @@ import {expect} from 'chai' import {restore, SinonStub, stub} from 'sinon' import Cmd from '../../../src/commands/logs.js' -import {LogDisplayer} from '../../../src/lib/run/log-displayer.js' describe('logs', function () { let logDisplayerStub: SinonStub beforeEach(async function () { - // Stub only the display method - logDisplayerStub = stub(LogDisplayer.prototype, 'display').resolves() + logDisplayerStub = stub(Cmd, 'displayLogs').resolves() }) afterEach(function () { diff --git a/test/unit/lib/run/log-displayer.unit.test.ts b/test/unit/lib/run/log-displayer.unit.test.ts index 08fb4a43b5..4355946df9 100644 --- a/test/unit/lib/run/log-displayer.unit.test.ts +++ b/test/unit/lib/run/log-displayer.unit.test.ts @@ -1,412 +1,74 @@ -/* eslint-disable unicorn/prefer-add-event-listener */ -import {APIClient} from '@heroku-cli/command' -import {captureOutput} from '@heroku-cli/test-utils' -import {Config, Errors} from '@oclif/core' import {expect} from 'chai' -import nock from 'nock' -import {SinonStub, stub} from 'sinon' -import tsheredoc from 'tsheredoc' +import {restore, type SinonStub, stub} from 'sinon' -import {LogDisplayer} from '../../../../src/lib/run/log-displayer.js' -import {cedarApp, firApp} from '../../../fixtures/apps/fixtures.js' +import {displayLogs} from '../../../../src/lib/run/log-displayer.js' -type CLIError = Errors.CLIError -const heredoc = tsheredoc.default - -describe('logDisplayer', function () { - let api: nock.Scope - let heroku: APIClient - let env: NodeJS.ProcessEnv - let displayer: LogDisplayer - let createEventSourceStub: SinonStub - - before(async function () { - env = process.env - env.HEROKU_LOGS_COLOR = '0' - const config = await Config.load() - heroku = new APIClient(config) - }) +describe('displayLogs', function () { + let streamLogsStub: SinonStub beforeEach(function () { - // Create a mock EventSource class - class MockEventSource { - public onerror: ((event: any) => void) | null = null - public onmessage: ((event: any) => void) | null = null - public onopen: ((event: any) => void) | null = null - public readyState: number = 0 // CONNECTING - public url: string - private errorCode: number - private timeouts: NodeJS.Timeout[] = [] - - constructor(url: string, options?: any) { - this.url = url - this.errorCode = 401 - - // Determine behavior based on URL - if (url.includes('telemetry.heroku.com')) { - // For Fir apps (telemetry URLs), return 500 error - this.errorCode = 500 - } - - // Simulate connection attempt - const timeout1 = setTimeout(() => { - // Check if this is the test that expects success (specific URL pattern for non-tail mode) - const isSuccessTest = this.url.includes('logs.heroku.com') && this.url.includes('tail=false') - - if (isSuccessTest) { - // Simulate successful connection - if (this.onopen) { - this.onopen({type: 'open'}) - } - - // Simulate log messages - if (this.onmessage) { - const messageEvent1 = { - data: '2024-10-17T22:23:22.209776+00:00 app[web.1]: log line 1\n\n\n', - type: 'message', - } - this.onmessage(messageEvent1) - - const messageEvent2 = { - data: '2024-10-17T22:23:23.032789+00:00 app[web.1]: log line 2\n\n\n', - type: 'message', - } - this.onmessage(messageEvent2) - } - - // Close after sending messages - const timeout2 = setTimeout(() => { - this.close() - // For non-tail mode, trigger error event to resolve the promise - if (this.onerror) { - const closeEvent = { - code: undefined, - type: 'error', - } - this.onerror(closeEvent) - } - }, 20) - this.timeouts.push(timeout2) - } else if (this.onerror) { - // Create a mock error event with status code - const errorEvent = { - code: this.errorCode, - type: 'error', - } - this.onerror(errorEvent) - } - }, 10) - this.timeouts.push(timeout1) - } - - addEventListener(type: string, listener: (event: any) => void) { - switch (type) { - case 'error': { - this.onerror = listener - break - } - - case 'message': { - this.onmessage = listener - break - } - - case 'open': { - this.onopen = listener - break - } - } - } - - close() { - this.readyState = 2 // CLOSED - // Clear all pending timeouts to prevent them from running after tests complete - for (const timeout of this.timeouts) { - clearTimeout(timeout) - } - - this.timeouts = [] - } - } - - // Create LogDisplayer instance - displayer = new LogDisplayer(heroku) - - // Stub the createEventSourceInstance method - createEventSourceStub = stub(displayer, 'createEventSourceInstance').callsFake((url: string, options?: any) => new MockEventSource(url, options) as any) + // displayLogs constructs `new HerokuSDK({extensions: [logSessionExtensions]})` + // and calls `.platform.logSession.streamLogs(app, options)`. Stub the + // extension factory's streamLogs method so we can assert on the args + // without making any network calls. + // + // logSessionExtensions.factory(ctx) is invoked once per HerokuSDK + // construction, returning {streamLogs: ...}. Wrap the factory so the + // returned object's streamLogs is our stub. + + streamLogsStub = stub().returns((async function * () {/* no-op */})()) }) afterEach(function () { - api?.done() - if (createEventSourceStub) { - createEventSourceStub.restore() - } - - // Ensure displayer is cleaned up to avoid hanging - if (displayer) { - displayer = null as any - } - }) - - after(function () { - process.env = env + restore() }) - describe('log session creation', function () { - context('with a Cedar app', function () { - beforeEach(function () { - api = nock('https://api.heroku.com', { - reqheaders: {Accept: 'application/vnd.heroku+json; version=3.sdk'}, - }).get('/apps/my-cedar-app') - .reply(200, cedarApp) - }) - - afterEach(function () { - api.done() - }) - - context('with dyno and no type options', function () { - it('creates a log session with dyno parameter set to the option value', async function () { - api - .post('/apps/my-cedar-app/log-sessions', { - dyno: 'web.1', - lines: 20, - source: 'app', - tail: true, - }) - .reply(200, {logplex_url: 'https://logs.heroku.com/stream?tail=true&token=s3kr3t'}) - - try { - await displayer.display({ - app: 'my-cedar-app', - dyno: 'web.1', - lines: 20, - source: 'app', - tail: true, - }) - } catch (error: unknown) { - const {message} = error as CLIError - expect(message).to.equal('Logs eventsource failed with: 401') - } - }) - }) - - context('with type and no dyno options', function () { - it('creates a log session with dyno parameter set to the type option value', async function () { - api - .post('/apps/my-cedar-app/log-sessions', { - dyno: 'web', - lines: 20, - source: 'app', - tail: true, - }) - .reply(200, {logplex_url: 'https://logs.heroku.com/stream?tail=true&token=s3kr3t'}) - - try { - await displayer.display({ - app: 'my-cedar-app', - lines: 20, - source: 'app', - tail: true, - type: 'web', - }) - } catch (error: unknown) { - const {message} = error as CLIError - expect(message).to.equal('Logs eventsource failed with: 401') - } - }) - }) - - context('with both type and dyno options', function () { - it('creates a log session with dyno parameter set to the option value, ignoring type', async function () { - api - .post('/apps/my-cedar-app/log-sessions', { - dyno: 'web.1', - lines: 20, - source: 'app', - tail: true, - }) - .reply(200, {logplex_url: 'https://logs.heroku.com/stream?tail=true&token=s3kr3t'}) - - try { - await displayer.display({ - app: 'my-cedar-app', - dyno: 'web.1', - lines: 20, - source: 'app', - tail: true, - type: 'web', - }) - } catch (error: unknown) { - const {message} = error as CLIError - expect(message).to.equal('Logs eventsource failed with: 401') - } - }) - }) + function patchExtension(stubFn: SinonStub) { + // The SDK's logSessionExtensions is an object with a factory function. + // Replace its factory to return {streamLogs: stubFn} when invoked, + // bypassing the real SDK plumbing for this unit test. + return import('@heroku/sdk/extensions/platform').then(mod => { + stub(mod.logSessionExtensions, 'factory').returns({streamLogs: stubFn} as never) }) - - context('with a Fir app and both lines and tail options present', function () { - beforeEach(function () { - api = nock('https://api.heroku.com', { - reqheaders: {Accept: 'application/vnd.heroku+json; version=3.sdk'}, - }).get('/apps/my-fir-app') - .reply(200, firApp) - }) - - it('creates a session with parameters set to option values, ignoring lines and tail options', async function () { - api.post('/apps/my-fir-app/log-sessions', { - dyno: 'web-123-456', - source: 'app', - type: 'web', - }) - .reply(200, {logplex_url: 'https://telemetry.heroku.com/streams/hyacinth-vbx?token=s3kr3t'}) - - try { - await displayer.display({ - app: 'my-fir-app', - dyno: 'web-123-456', - lines: 20, - source: 'app', - tail: true, - type: 'web', - }) - } catch (error: unknown) { - const {message} = error as CLIError - expect(message.trim()).to.equal('Logs eventsource failed with: 500') - } - }) - }) - }) - - context('with a Cedar app, with tail option disabled', function () { - beforeEach(function () { - api = nock('https://api.heroku.com', { - reqheaders: {Accept: 'application/vnd.heroku+json; version=3.sdk'}, - }).get('/apps/my-cedar-app') - .reply(200, cedarApp) - .post('/apps/my-cedar-app/log-sessions', {tail: false}) - .reply(200, {logplex_url: 'https://logs.heroku.com/stream?tail=false&token=s3kr3t'}) + } + + it('forwards command options as streamLogs options', async function () { + await patchExtension(streamLogsStub) + + await displayLogs({ + app: 'my-app', + dyno: 'web.1', + lines: 50, + source: 'app', + tail: true, + type: undefined, }) - afterEach(function () { - api.done() - }) - - context('when the log server returns an error', function () { - it('shows the error and exits', async function () { - try { - await displayer.display({ - app: 'my-cedar-app', - tail: false, - }) - } catch (error: unknown) { - const {message, oclif} = error as CLIError - expect(message).to.equal('Logs eventsource failed with: 401') - expect(oclif.exit).to.eq(1) - } - }) - }) - - context('when the log server responds with a stream of log lines', function () { - it('displays log lines and exits', async function () { - nock('https://logs.heroku.com', { - reqheaders: {Accept: 'text/event-stream'}, - }).get('/stream') - .query(true) - .reply(200, heredoc` - id: 1002 - data: 2024-10-17T22:23:22.209776+00:00 app[web.1]: log line 1\n\n\n - id: 1003 - data: 2024-10-17T22:23:23.032789+00:00 app[web.1]: log line 2\n\n\n - `) - - const {stdout} = await captureOutput(async () => { - await displayer.display({ - app: 'my-cedar-app', - tail: false, - }) - }) - - // Note: logServer.done() is not called because our MockEventSource intercepts the request - expect(stdout).to.eq(heredoc` - 2024-10-17T22:23:22.209776+00:00 app[web.1]: log line 1 - 2024-10-17T22:23:23.032789+00:00 app[web.1]: log line 2 - `) - }) - }) - }) - - context('with a Cedar app, with tail option enabled', function () { - beforeEach(function () { - api = nock('https://api.heroku.com', { - reqheaders: {Accept: 'application/vnd.heroku+json; version=3.sdk'}, - }).get('/apps/my-cedar-app') - .reply(200, cedarApp) - .post('/apps/my-cedar-app/log-sessions', {tail: true}) - .reply(200, {logplex_url: 'https://logs.heroku.com/stream?tail=true&token=s3kr3t'}) - }) - - afterEach(function () { - api.done() - }) - - context('when the log server returns an error', function () { - it('shows the error and exits', async function () { - try { - await displayer.display({ - app: 'my-cedar-app', - tail: true, - }) - } catch (error: unknown) { - const {message, oclif} = error as CLIError - expect(message).to.equal('Logs eventsource failed with: 401') - expect(oclif.exit).to.eq(1) - } - }) - }) - - context('when the log server responds with a stream of log lines and then timeouts', function () { - it('displays log lines and exits showing a timeout error', async function () { - try { - await displayer.display({ - app: 'my-cedar-app', - tail: true, - }) - expect.fail('Expected error to be thrown') - } catch (error: unknown) { - const {message, oclif} = error as CLIError - expect(message).to.equal('Logs eventsource failed with: 401') - expect(oclif.exit).to.eq(1) - } - }) - }) + expect(streamLogsStub.calledOnce).to.be.true + expect(streamLogsStub.firstCall.args[0]).to.equal('my-app') + const passedOptions = streamLogsStub.firstCall.args[1] + expect(passedOptions.dyno).to.equal('web.1') + expect(passedOptions.lines).to.equal(50) + expect(passedOptions.source).to.equal('app') + expect(passedOptions.tail).to.equal(true) + expect(passedOptions.type).to.equal(undefined) + expect(passedOptions.signal).to.be.an.instanceOf(AbortSignal) + expect(passedOptions.onSessionCreated).to.be.a('function') }) - context('with a Fir app', function () { - beforeEach(function () { - api = nock('https://api.heroku.com', { - reqheaders: {Accept: 'application/vnd.heroku+json; version=3.sdk'}, - }).get('/apps/my-fir-app') - .reply(200, firApp) - .post('/apps/my-fir-app/log-sessions') - .reply(500) - }) + it('passes through optional fields as undefined when not provided', async function () { + await patchExtension(streamLogsStub) - afterEach(function () { - api.done() + await displayLogs({ + app: 'my-app', + tail: false, }) - it('displays logs and recreates log sessions on timeout', async function () { - try { - await displayer.display({ - app: 'my-fir-app', - tail: false, - }) - expect.fail('Expected error to be thrown') - } catch (error: unknown) { - const {message} = error as Error - expect(message.trim()).to.equal('HTTP Error 500 for POST https://api.heroku.com/apps/my-fir-app/log-sessions') - } - }) + const passedOptions = streamLogsStub.firstCall.args[1] + expect(passedOptions.dyno).to.equal(undefined) + expect(passedOptions.lines).to.equal(undefined) + expect(passedOptions.source).to.equal(undefined) + expect(passedOptions.type).to.equal(undefined) + expect(passedOptions.tail).to.equal(false) }) }) From fd0135a45f51bf0077fa68868c5061b6f06fb410 Mon Sep 17 00:00:00 2001 From: Eric Black Date: Fri, 22 May 2026 11:57:18 -0700 Subject: [PATCH 5/7] chore: drop unused eventsource dep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The legacy LogDisplayer used EventSource to consume the logplex stream. With the SDK migration, that's gone — the stream is read directly as chunked text/plain, which is what the platform actually serves. No remaining imports of 'eventsource' anywhere in src/. --- package-lock.json | 13 ------------- package.json | 1 - 2 files changed, 14 deletions(-) diff --git a/package-lock.json b/package-lock.json index c178abfeee..ccc54e7358 100644 --- a/package-lock.json +++ b/package-lock.json @@ -48,7 +48,6 @@ "commander": "^2.15.1", "date-fns": "^2.30.0", "debug": "^4.4.3", - "eventsource": "^4", "execa": "^9.6.1", "filesize": "^10.1", "foreman": "^3.0.1", @@ -13514,18 +13513,6 @@ "bare-events": "^2.7.0" } }, - "node_modules/eventsource": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/eventsource/-/eventsource-4.1.0.tgz", - "integrity": "sha512-2GuF51iuHX6A9xdTccMTsNb7VO0lHZihApxhvQzJB5A03DvHDd2FQepodbMaztPBmBcE/ox7o2gqaxGhYB9LhQ==", - "license": "MIT", - "dependencies": { - "eventsource-parser": "^3.0.1" - }, - "engines": { - "node": ">=20.0.0" - } - }, "node_modules/eventsource-parser": { "version": "3.0.6", "resolved": "https://registry.npmjs.org/eventsource-parser/-/eventsource-parser-3.0.6.tgz", diff --git a/package.json b/package.json index 1e303625bf..d7b206c766 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,6 @@ "commander": "^2.15.1", "date-fns": "^2.30.0", "debug": "^4.4.3", - "eventsource": "^4", "execa": "^9.6.1", "filesize": "^10.1", "foreman": "^3.0.1", From 54cea2cc85ab9e95e4e843961aa8006040f9cafc Mon Sep 17 00:00:00 2001 From: Eric Black Date: Fri, 22 May 2026 15:12:59 -0700 Subject: [PATCH 6/7] deps: point @heroku/sdk back at main heroku/heroku-sdk#28 (logSession.streamLogs) has merged. Picks up @heroku/heroku-fetch@bf6be07 transitively (the merged dispatcher fix plus the proxy/Accept/ky2 work). --- package-lock.json | 26 +++++++++++++------------- package.json | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/package-lock.json b/package-lock.json index ccc54e7358..6f842d9de8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,7 +16,7 @@ "@heroku/heroku-cli-util": "^10.8.0", "@heroku/http-call": "^5.5.1", "@heroku/mcp-server": "^1.2.0", - "@heroku/sdk": "github:heroku/heroku-sdk#eb/feat/stream-logs", + "@heroku/sdk": "github:heroku/heroku-sdk#main", "@heroku/socksv5": "^0.0.9", "@inquirer/prompts": "^7.0", "@oclif/core": "^4.8.4", @@ -2575,11 +2575,12 @@ }, "node_modules/@heroku/heroku-fetch": { "version": "0.1.0", - "resolved": "git+ssh://git@github.com/heroku/heroku-fetch.git#90940bdb0191cafcd9571492b2145980643ba7f0", + "resolved": "git+ssh://git@github.com/heroku/heroku-fetch.git#bf6be077a9186c19d1327dad8c3709770324b2a2", "license": "Apache-2.0", "dependencies": { "debug": "^4.3.4", - "ky": "^1.2.0" + "ky": "^2.0.2", + "undici": "^6.25.0" }, "engines": { "node": ">=22" @@ -2667,10 +2668,10 @@ }, "node_modules/@heroku/sdk": { "version": "0.4.0", - "resolved": "git+ssh://git@github.com/heroku/heroku-sdk.git#a369965cec1d4f374802e5da52c721deec1cb945", + "resolved": "git+ssh://git@github.com/heroku/heroku-sdk.git#19e799844a066c17b32a137840ce4630f18215d2", "license": "Apache-2.0", "dependencies": { - "@heroku/heroku-fetch": "github:heroku/heroku-fetch#eb/feat/proxy-support", + "@heroku/heroku-fetch": "github:heroku/heroku-fetch#main", "@heroku/types": "github:heroku/heroku-types", "debug": "^4.4.0" }, @@ -16697,12 +16698,12 @@ } }, "node_modules/ky": { - "version": "1.14.3", - "resolved": "https://registry.npmjs.org/ky/-/ky-1.14.3.tgz", - "integrity": "sha512-9zy9lkjac+TR1c2tG+mkNSVlyOpInnWdSMiue4F+kq8TwJSgv6o8jhLRg8Ho6SnZ9wOYUq/yozts9qQCfk7bIw==", + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/ky/-/ky-2.0.2.tgz", + "integrity": "sha512-/GmXpo9F9W+f8n4Ivr2iH+7h7wL7jLbLKWkMlpflcCRb6kGjBfTlASEXaZ9qUgNTn4VgS0P2pwxxzQ4EM6Ulgg==", "license": "MIT", "engines": { - "node": ">=18" + "node": ">=22" }, "funding": { "url": "https://github.com/sindresorhus/ky?sponsor=1" @@ -23512,10 +23513,9 @@ } }, "node_modules/undici": { - "version": "6.24.1", - "resolved": "https://registry.npmjs.org/undici/-/undici-6.24.1.tgz", - "integrity": "sha512-sC+b0tB1whOCzbtlx20fx3WgCXwkW627p4EA9uM+/tNNPkSS+eSEld6pAs9nDv7WbY1UUljBMYPtu9BCOrCWKA==", - "dev": true, + "version": "6.25.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-6.25.0.tgz", + "integrity": "sha512-ZgpWDC5gmNiuY9CnLVXEH8rl50xhRCuLNA97fAUnKi8RRuV4E6KG31pDTsLVUKnohJE0I3XDrTeEydAXRw47xg==", "license": "MIT", "engines": { "node": ">=18.17" diff --git a/package.json b/package.json index d7b206c766..30e2b8a7a0 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "@heroku/heroku-cli-util": "^10.8.0", "@heroku/http-call": "^5.5.1", "@heroku/mcp-server": "^1.2.0", - "@heroku/sdk": "github:heroku/heroku-sdk#eb/feat/stream-logs", + "@heroku/sdk": "github:heroku/heroku-sdk#main", "@heroku/socksv5": "^0.0.9", "@inquirer/prompts": "^7.0", "@oclif/core": "^4.8.4", From 288fd442a8ec780ee34d61434eb78839a467cf20 Mon Sep 17 00:00:00 2001 From: Eric Black Date: Fri, 22 May 2026 15:19:47 -0700 Subject: [PATCH 7/7] fix(log-displayer): hoist EPIPE listener to module load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit setupProcessHandlers ran on every displayLogs() call and added a fresh process.stdout 'error' listener with no removal. Single-shot CLI invocations don't notice, but repeated calls in-process would accumulate listeners and trip Node's MaxListeners warning. Move the listener to module load (it's already process-global state either way) so each call wires only the per-invocation SIGINT/SIGTERM abort handlers. Also surface err.message ?? String(err) instead of err.stack — err.stack can be undefined on non-Error throwables. Adds a unit test for the abort-swallow path: when streamLogs rejects with an AbortError after we abort the controller, displayLogs should resolve cleanly. --- src/lib/run/log-displayer.ts | 25 ++++++++++---------- test/unit/lib/run/log-displayer.unit.test.ts | 22 +++++++++++++++++ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/lib/run/log-displayer.ts b/src/lib/run/log-displayer.ts index 43f5d38b04..fcdf50b9d0 100644 --- a/src/lib/run/log-displayer.ts +++ b/src/lib/run/log-displayer.ts @@ -14,9 +14,19 @@ export interface LogDisplayerOptions { type?: string } -export async function displayLogs(options: LogDisplayerOptions): Promise { - setupProcessHandlers() +// Install once at module load so repeated displayLogs() calls don't +// stack listeners on process.stdout (which would trip Node's +// MaxListeners warning). +process.stdout.on('error', err => { + if (err.code === 'EPIPE') { + // eslint-disable-next-line n/no-process-exit, unicorn/no-process-exit + process.exit(0) + } else { + ux.error(err.message ?? String(err), {exit: 1}) + } +}) +export async function displayLogs(options: LogDisplayerOptions): Promise { const controller = new AbortController() const onAbort = () => controller.abort() process.once('SIGINT', onAbort) @@ -52,14 +62,3 @@ export async function displayLogs(options: LogDisplayerOptions): Promise { process.off('SIGTERM', onAbort) } } - -function setupProcessHandlers(): void { - process.stdout.on('error', err => { - if (err.code === 'EPIPE') { - // eslint-disable-next-line n/no-process-exit, unicorn/no-process-exit - process.exit(0) - } else { - ux.error(err.stack, {exit: 1}) - } - }) -} diff --git a/test/unit/lib/run/log-displayer.unit.test.ts b/test/unit/lib/run/log-displayer.unit.test.ts index 4355946df9..b929845802 100644 --- a/test/unit/lib/run/log-displayer.unit.test.ts +++ b/test/unit/lib/run/log-displayer.unit.test.ts @@ -71,4 +71,26 @@ describe('displayLogs', function () { expect(passedOptions.type).to.equal(undefined) expect(passedOptions.tail).to.equal(false) }) + + it('swallows the rejection when the stream throws after abort', async function () { + // Mimic streamLogs throwing an AbortError after the caller aborts. + // displayLogs should resolve cleanly rather than re-throw. + const abortAwareStub = stub().callsFake((_app: string, opts: {signal: AbortSignal}) => (async function * () { + await new Promise((resolve, reject) => { + opts.signal.addEventListener('abort', () => { + const err = new Error('aborted') + err.name = 'AbortError' + reject(err) + }, {once: true}) + }) + yield '' + })()) + + await patchExtension(abortAwareStub) + + const displayPromise = displayLogs({app: 'my-app', tail: true}) + process.emit('SIGINT') + + await displayPromise + }) })