refactor: migrate LogDisplayer to @heroku/sdk streamLogs

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",

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)
+    })
   }
 }

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() {

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,

src/lib/run/log-displayer.ts

@@ -1,174 +1,64 @@
-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 {
-  app: string,
+export interface LogDisplayerOptions {
+  app: string
   dyno?: string
   lines?: number
   source?: string
   tail: boolean
   type?: string
 }
 
-export class LogDisplayer {
-  private heroku: APIClient
-
-  constructor(heroku: APIClient) {
-    this.heroku = heroku
-  }
-
-  public createEventSourceInstance(url: string, options?: any): EventSource {
-    return new EventSource(url, options)
-  }
-
-  async display(options: LogDisplayerOptions): Promise<void> {
-    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)
+// 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})
   }
-
-  private buildRequestBodyParameters(firApp: boolean, options: LogDisplayerOptions): Record<string, any> {
-    const requestBodyParameters = {
+})
+
+export async function displayLogs(options: LogDisplayerOptions): Promise<void> {
+  const controller = new AbortController()
+  const onAbort = () => controller.abort()
+  process.once('SIGINT', onAbort)
+  process.once('SIGTERM', onAbort)
+
+  const {platform} = new HerokuSDK({extensions: [logSessionExtensions]})
+
+  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))
     }
-
-    if (firApp) {
-      process.stderr.write(color.info('Fetching logs...\n\n'))
-      Object.assign(requestBodyParameters, {
-        dyno: options.dyno,
-        type: options.type,
-      })
-    } else {
-      Object.assign(requestBodyParameters, {
-        dyno: options.dyno || options.type,
-        lines: options.lines,
-        tail: options.tail,
-      })
-    }
-
-    return requestBodyParameters
-  }
-
-  private async createLogSession(requestBodyParameters: Record<string, any>, app: string): Promise<LogSession> {
-    const {body: logSession} = await this.heroku.post<LogSession>(`/apps/${app}/log-sessions`, {
-      body: requestBodyParameters,
-      headers: {Accept: 'application/vnd.heroku+json; version=3.sdk'},
-    })
-    return logSession
-  }
-
-  private async getGenerationByAppId(options: LogDisplayerOptions): Promise<string> {
-    const generation = await getGenerationByAppId(options.app, this.heroku)
-    return generation || ''
-  }
-
-  private readLogs(logplexURL: string, isTail: boolean, recreateSessionTimeout?: number): Promise<void> {
-    return new Promise<void>((resolve, reject) => {
-      const userAgent = process.env.HEROKU_DEBUG_USER_AGENT || 'heroku-run'
-      const proxy = process.env.https_proxy || process.env.HTTPS_PROXY
-
-      // 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 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)
-      }
-    })
+  } 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)
   }
-
-  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})
-      }
-    })
-  }
-}
-
-// Default export for backward compatibility
-export default async function logDisplayer(heroku: APIClient, options: LogDisplayerOptions): Promise<void> {
-  const displayer = new LogDisplayer(heroku)
-  await displayer.display(options)
 }

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 () {