Skip to content

Commit

Permalink
WIP Add timeout to heroku run
Browse files Browse the repository at this point in the history
The library (Hatchet) that buildpacks use to integration test against Heroku shells out to `heroku run` where we're wanting to be able to configure a timeout that's less than 60 minutes. Instead of working around this on the library side, I think it would make more sense to add a configurable timeout to the cli.

This PR against Hatchet explains the issue heroku/hatchet#158. 


This is a WIP as I'm not very familiar with the CLI or it's development workflow at all. I got as far as getting it to trigger an exception:

```
$ DEBUG=* ./bin/run run echo lol -a issuetriage -timeout 5
# ...
Running echo lol -timeout 5 on ⬢ issuetriage... done
  heroku:run RangeError [ERR_OUT_OF_RANGE] [ERR_OUT_OF_RANGE]: The value of "msecs" is out of range. It must be a non-negative finite number. Received NaN
  heroku:run     at new NodeError (node:internal/errors:329:5)
  heroku:run     at getTimerDuration (node:internal/timers:386:11)
  heroku:run     at TLSSocket.setStreamTimeout [as setTimeout] (node:internal/stream_base_commons:252:11)
  heroku:run     at /Users/rschneeman/Documents/projects/work/cli/packages/run/src/lib/dyno.ts:126:15
  heroku:run     at new Promise (<anonymous>)
  heroku:run     at Dyno._rendezvous (/Users/rschneeman/Documents/projects/work/cli/packages/run/src/lib/dyno.ts:117:16)
  heroku:run     at Dyno.attach (/Users/rschneeman/Documents/projects/work/cli/packages/run/src/lib/dyno.ts:110:27)
  heroku:run     at Dyno._doStart (/Users/rschneeman/Documents/projects/work/cli/packages/run/src/lib/dyno.ts:77:28)
  heroku:run     at processTicksAndRejections (node:internal/process/task_queues:94:5)
  heroku:run     at Dyno.start (/Users/rschneeman/Documents/projects/work/cli/packages/run/src/lib/dyno.ts:54:9)
  heroku:run     at Run.run (/Users/rschneeman/Documents/projects/work/cli/packages/run/src/commands/run/index.ts:36:13)
  heroku:run     at Run._run (/Users/rschneeman/Documents/projects/work/cli/node_modules/@oclif/command/lib/command.js:44:20)
  heroku:run     at Config.runCommand (/Users/rschneeman/Documents/projects/work/cli/node_modules/@oclif/config/lib/config.js:151:9)
  heroku:run     at Main.run (/Users/rschneeman/Documents/projects/work/cli/node_modules/@oclif/command/lib/main.js:21:9)
  heroku:run     at Main._run (/Users/rschneeman/Documents/projects/work/cli/node_modules/@oclif/command/lib/command.js:44:20) +0ms
RangeError [ERR_OUT_OF_RANGE] [ERR_OUT_OF_RANGE]: The value of "msecs" is out of range. It must be a non-negative finite number. Received NaN
    at new NodeError (node:internal/errors:329:5)
    at getTimerDuration (node:internal/timers:386:11)
    at TLSSocket.setStreamTimeout [as setTimeout] (node:internal/stream_base_commons:252:11)
    at ~/Documents/projects/work/cli/packages/run/src/lib/dyno.ts:126:15
    at new Promise (<anonymous>)
    at Dyno._rendezvous (~/Documents/projects/work/cli/packages/run/src/lib/dyno.ts:117:16)
    at Dyno.attach (~/Documents/projects/work/cli/packages/run/src/lib/dyno.ts:110:27)
    at Dyno._doStart (~/Documents/projects/work/cli/packages/run/src/lib/dyno.ts:77:28)
    at processTicksAndRejections (node:internal/process/task_queues:94:5)
    at Dyno.start (~/Documents/projects/work/cli/packages/run/src/lib/dyno.ts:54:9)
    at Run.run (~/Documents/projects/work/cli/packages/run/src/commands/run/index.ts:36:13)
    at Run._run (~/Documents/projects/work/cli/node_modules/@oclif/command/lib/command.js:44:20)
    at Config.runCommand (~/Documents/projects/work/cli/node_modules/@oclif/config/lib/config.js:151:9)
    at Main.run (~/Documents/projects/work/cli/node_modules/@oclif/command/lib/main.js:21:9)
    at Main._run (~/Documents/projects/work/cli/node_modules/@oclif/command/lib/command.js:44:20)
⛄ 3.0.1 🚀  ~/Documents/projects/work/cli (schneems/timeout-lime-out)
$ gitx .
⛄ 3.0.1 🚀  ~/Documents/projects/work/cli (schneems/timeout-lime-out)
```
  • Loading branch information
schneems committed May 21, 2021
1 parent 4b89482 commit 46421cb
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 4 deletions.
6 changes: 4 additions & 2 deletions packages/run-v5/commands/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ async function run (context, heroku) {
env: context.flags.env,
'no-tty': context.flags['no-tty'],
attach: true,
listen: context.flags.listen
listen: context.flags.listen,
timeout: context.flags['timeout']
}
if (!opts.command) throw new Error('Usage: heroku run COMMAND\n\nExample: heroku run bash')

Expand Down Expand Up @@ -53,7 +54,8 @@ Running myscript.sh -a arg1 -s arg2 on app.... up, run.1`,
{ name: 'env', char: 'e', description: "environment variables to set (use ';' to split multiple vars)", hasValue: true },
{ name: 'no-tty', description: 'force the command to not run in a tty', hasValue: false },
{ name: 'listen', description: 'listen on a local port', hasValue: false, hidden: true },
{ name: 'no-notify', description: 'disables notification when dyno is up (alternatively use HEROKU_NOTIFICATIONS=0)', hasValue: false }
{ name: 'no-notify', description: 'disables notification when dyno is up (alternatively use HEROKU_NOTIFICATIONS=0)', hasValue: false },
{ name: 'timeout', int: 3600, description: 'sets a timeout on the command in seconds', hasValue: true }
],
run: cli.command(run)
}
3 changes: 2 additions & 1 deletion packages/run-v5/lib/dyno.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class Dyno extends Duplex {
this.cork()
this.opts = opts
this.heroku = opts.heroku
this.timeout = opts.timeout
if (this.opts.showStatus === undefined) this.opts.showStatus = true
}

Expand Down Expand Up @@ -114,7 +115,7 @@ class Dyno extends Duplex {

if (this.opts.showStatus) cli.action.status(this._status('starting'))
let c = tls.connect(this.uri.port, this.uri.hostname, {rejectUnauthorized: this.heroku.options.rejectUnauthorized})
c.setTimeout(1000 * 60 * 60)
c.setTimeout(1000 * timeout)
c.setEncoding('utf8')
c.on('connect', () => {
debug('connect')
Expand Down
2 changes: 2 additions & 0 deletions packages/run/src/commands/run/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export default class Run extends Command {
'no-tty': flags.boolean({description: 'force the command to not run in a tty'}),
listen: flags.boolean({description: 'listen on a local port', hidden: true}),
'no-notify': flags.boolean({description: 'disables notification when dyno is up (alternatively use HEROKU_NOTIFICATIONS=0)'}),
timeout: flags.boolean({description: 'sets a timeout on the command in seconds'}),
}

async run() {
Expand All @@ -45,6 +46,7 @@ export default class Run extends Command {
heroku: this.heroku,
listen: flags.listen,
notify: !flags['no-notify'],
timeout: flags.timeout,
size: flags.size,
type: flags.type,
}
Expand Down
3 changes: 2 additions & 1 deletion packages/run/src/lib/dyno.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ interface DynoOpts {
showStatus?: boolean;
size?: string;
type?: string;
timeout: number;
}

export default class Dyno extends Duplex {
Expand Down Expand Up @@ -169,7 +170,7 @@ export default class Dyno extends Duplex {
const c = tls.connect(parseInt(this.uri.port, 10), this.uri.hostname, {
rejectUnauthorized: this.heroku.options.rejectUnauthorized,
})
c.setTimeout(1000 * 60 * 60)
c.setTimeout(1000 * this.opts.timeout)
c.setEncoding('utf8')
c.on('connect', () => {
debug('connect')
Expand Down

0 comments on commit 46421cb

Please sign in to comment.