Skip to content

Commit

Permalink
fix(run): fix run status code and use https (#1351)
Browse files Browse the repository at this point in the history
  • Loading branch information
cafreeman committed May 6, 2020
1 parent 3ef7e11 commit ca8127f
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 43 deletions.
6 changes: 3 additions & 3 deletions .circleci/config.yml
Expand Up @@ -19,7 +19,7 @@ jobs:
windows-test:
executor:
name: win/default
size: large
size: xlarge
steps:
- checkout
- run:
Expand Down Expand Up @@ -50,7 +50,7 @@ jobs:
command: yarn test
node12-test: &test
<<: *defaults
resource_class: large
resource_class: xlarge
steps:
- checkout
- restore_cache: &yarn_restore_cache
Expand Down Expand Up @@ -80,7 +80,7 @@ jobs:
<<: *test
docker:
- image: heroku/nsis:10-2
resource_class: large
resource_class: xlarge
release_tarballs:
<<: *defaults
steps:
Expand Down
3 changes: 1 addition & 2 deletions package.json
Expand Up @@ -22,8 +22,7 @@
"private": true,
"scripts": {
"test": "lerna run test --concurrency 4",
"version": "cp packages/cli/CHANGELOG.md CHANGELOG.md && git add CHANGELOG.md",
"posttest": "standard"
"version": "cp packages/cli/CHANGELOG.md CHANGELOG.md && git add CHANGELOG.md"
},
"workspaces": [
"packages/*"
Expand Down
4 changes: 2 additions & 2 deletions packages/local/bin/bats-test-runner.js
@@ -1,7 +1,7 @@
#!/usr/bin/env node

const os = require('os')
const { spawn } = require('child_process')
const {spawn} = require('child_process')

if (os.platform() === 'win32' || os.platform() === 'windows') console.log('skipping on windows')
else spawn('npx bats test/integration/*.bats', { stdio: 'inherit', shell: true })
else spawn('npx bats test/integration/*.bats', {stdio: 'inherit', shell: true})
7 changes: 7 additions & 0 deletions packages/run/bin/bats-test-runner.js
@@ -0,0 +1,7 @@
#!/usr/bin/env node

const os = require('os')
const {spawn} = require('child_process')

if (os.platform() === 'win32' || os.platform() === 'windows') console.log('skipping on windows')
else spawn('npx bats test/integration/*.bats', {stdio: 'inherit', shell: true})
5 changes: 4 additions & 1 deletion packages/run/package.json
Expand Up @@ -23,6 +23,7 @@
"@types/mocha": "^5",
"@types/node": "^10",
"@types/node-notifier": "^5.4.0",
"bats": "^1.1.0",
"chai": "^4",
"eslint": "^6.7.2",
"eslint-config-oclif": "^3.1.0",
Expand Down Expand Up @@ -69,7 +70,9 @@
"postpack": "rm -f oclif.manifest.json",
"prepack": "rm -rf lib && tsc -b && oclif-dev manifest && oclif-dev readme",
"pretest": "tsc -p test --noEmit",
"test": "nyc --extension .ts mocha --forbid-only \"test/**/*.test.ts\"",
"test": "npm run test:unit",
"test:unit": "nyc --extension .ts mocha --forbid-only \"test/**/*.test.ts\"",
"test:integration": "node ./bin/bats-test-runner",
"posttest": "yarn lint",
"version": "oclif-dev readme && git add README.md"
}
Expand Down
60 changes: 26 additions & 34 deletions packages/run/src/lib/dyno.ts
Expand Up @@ -6,8 +6,8 @@ import {Dyno as APIDyno} from '@heroku-cli/schema'
import {spawn} from 'child_process'
import cli from 'cli-ux'
import debugFactory from 'debug'
import * as http from 'http'
import {HTTP} from 'http-call'
import * as https from 'https'
import * as net from 'net'
import {Duplex, Transform} from 'stream'
import * as tls from 'tls'
Expand Down Expand Up @@ -95,37 +95,33 @@ export default class Dyno extends Duplex {
await this._doStart()
}

_doStart(retries = 2): Promise<HTTP<unknown>> {
async _doStart(retries = 2): Promise<HTTP<unknown>> {
const command = this.opts['exit-code'] ? `${this.opts.command}; echo "\uFFFF heroku-command-exit-status: $?"` : this.opts.command

return this.heroku.post(this.opts.dyno ? `/apps/${this.opts.app}/dynos/${this.opts.dyno}` : `/apps/${this.opts.app}/dynos`, {
headers: {
Accept: this.opts.dyno ? 'application/vnd.heroku+json; version=3.run-inside' : 'application/vnd.heroku+json; version=3',
},
body: {
command,
attach: this.opts.attach,
size: this.opts.size,
type: this.opts.type,
env: this._env(),
force_no_tty: this.opts['no-tty'],
},
})
.then(dyno => {
try {
const dyno = await this.heroku.post(this.opts.dyno ? `/apps/${this.opts.app}/dynos/${this.opts.dyno}` : `/apps/${this.opts.app}/dynos`, {
headers: {
Accept: this.opts.dyno ? 'application/vnd.heroku+json; version=3.run-inside' : 'application/vnd.heroku+json; version=3',
},
body: {
command,
attach: this.opts.attach,
size: this.opts.size,
type: this.opts.type,
env: this._env(),
force_no_tty: this.opts['no-tty'],
},
})
this.dyno = dyno.body
if (this.opts.attach || this.opts.dyno) {
if (this.dyno.name && this.opts.dyno === undefined) {
this.opts.dyno = this.dyno.name
}

return this.attach()
}

if (this.opts.showStatus) {
await this.attach()
} else if (this.opts.showStatus) {
cli.action.stop(this._status('done'))
}
})
.catch(error => {
} catch (error) {
// Currently the runtime API sends back a 409 in the event the
// release isn't found yet. API just forwards this response back to
// the client, so we'll need to retry these. This usually
Expand All @@ -135,17 +131,13 @@ export default class Dyno extends Duplex {
if (error.statusCode === 409 && retries > 0) {
return this._doStart(retries - 1)
}

throw error
})
.finally(() => {
} finally {
cli.action.stop()
})
}
}

/**
* Attaches stdin/stdout to dyno
*/
// Attaches stdin/stdout to dyno
attach() {
this.pipe(process.stdout)
if (this.dyno && this.dyno.attach_url) {
Expand Down Expand Up @@ -206,7 +198,7 @@ export default class Dyno extends Duplex {
const interval = 1000

try {
const dyno = await this.heroku.get(`/apps/${this.opts.app}/dynos/${this.opts.dyno}`)
const {body: dyno} = await this.heroku.get(`/apps/${this.opts.app}/dynos/${this.opts.dyno}`)
this.dyno = dyno
cli.action.stop(this._status(this.dyno.state))

Expand All @@ -218,7 +210,7 @@ export default class Dyno extends Duplex {
return this._ssh()
} catch (error) {
// the API sometimes responds with a 404 when the dyno is not yet ready
if (error.statusCode === 404 && retries > 0) {
if (error.http.statusCode === 404 && retries > 0) {
return this._ssh(retries - 1)
}

Expand All @@ -231,10 +223,10 @@ export default class Dyno extends Duplex {
this.resolve = resolve
this.reject = reject

const options: http.RequestOptions & { rejectUnauthorized?: boolean } = this.uri
const options: https.RequestOptions & { rejectUnauthorized?: boolean } = this.uri
options.headers = {Connection: 'Upgrade', Upgrade: 'tcp'}
options.rejectUnauthorized = false
const r = http.request(options)
const r = https.request(options)
r.end()

r.on('error', this.reject)
Expand Down
6 changes: 6 additions & 0 deletions packages/run/test/integration/run.bats
@@ -0,0 +1,6 @@
@test "run with shield app" {
run ./bin/run run -a heroku-run-shield-test-app echo '1 2 3'
echo $output
[ "$status" -eq 0 ]
[[ "$output" =~ "1 2 3" ]]
}
2 changes: 1 addition & 1 deletion packages/run/test/mocha.opts
Expand Up @@ -2,4 +2,4 @@
--watch-extensions ts
--recursive
--reporter spec
--timeout 60000
--timeout 180000

0 comments on commit ca8127f

Please sign in to comment.