From ba09aeddbf7a42b87cad7dcfa1565cbd0d14bf96 Mon Sep 17 00:00:00 2001 From: Santiago Bosio Date: Wed, 14 Aug 2024 20:53:14 -0300 Subject: [PATCH] Adding missing debug and warning behaviors to APIClient --- src/api-client.ts | 17 ++++- src/command.ts | 8 +- test/api-client.test.ts | 159 ++++++++++++++++++++++++++++++++++++---- 3 files changed, 166 insertions(+), 18 deletions(-) diff --git a/src/api-client.ts b/src/api-client.ts index dcf8f17..817949a 100644 --- a/src/api-client.ts +++ b/src/api-client.ts @@ -11,6 +11,8 @@ import {RequestId, requestIdHeader} from './request-id' import {vars} from './vars' import {ParticleboardClient, IDelinquencyInfo, IDelinquencyConfig} from './particleboard-client' +const debug = require('debug') + export namespace APIClient { export interface Options extends HTTPRequestOptions { retryAuth?: boolean @@ -20,6 +22,8 @@ export namespace APIClient { export interface IOptions { required?: boolean preauth?: boolean + debug?: boolean + debugHeaders?: boolean } export interface IHerokuAPIErrorOptions { @@ -63,6 +67,8 @@ export class APIClient { if (options.required === undefined) options.required = true options.preauth = options.preauth !== false + if (options.debug) debug.enable('http') + if (options.debug && options.debugHeaders) debug.enable('http,http:headers') this.options = options const apiUrl = url.URL ? new url.URL(vars.apiUrl) : url.parse(vars.apiUrl) const envHeaders = JSON.parse(process.env.HEROKU_HEADERS || '{}') @@ -111,6 +117,14 @@ export class APIClient { } } + static showWarnings(response: HTTP) { + const warnings = response.headers['x-heroku-warning'] || response.headers['warning-message'] + if (Array.isArray(warnings)) + warnings.forEach(warning => warn(warning)) + else if (typeof warnings === 'string') + warn(warnings) + } + static configDelinquency(url: string, opts: APIClient.Options): void { if (opts.method?.toUpperCase() !== 'GET' || (opts.hostname && opts.hostname !== apiUrl.hostname)) { delinquencyConfig.fetch_delinquency = false @@ -153,7 +167,7 @@ export class APIClient { } if (deletion) - warn(`This ${resource} is delinquent with payment and we suspended it on ${new Date(suspension)}. If the ${resource} is still delinquent, we'll delete it on ${new Date(deletion)}.`) + warn(`This ${resource} is delinquent with payment and we suspended it on ${new Date(suspension)}. If the ${resource} is still delinquent, we‘ll delete it on ${new Date(deletion)}.`) } else if (deletion) warn(`This ${resource} is delinquent with payment and we‘ll delete it on ${new Date(deletion)}.`) @@ -201,6 +215,7 @@ export class APIClient { this.notifyDelinquency(delinquencyInfo) this.trackRequestIds(response) + this.showWarnings(response) return response } catch (error) { if (!(error instanceof deps.HTTP.HTTPError)) throw error diff --git a/src/command.ts b/src/command.ts index 764cd55..c051988 100644 --- a/src/command.ts +++ b/src/command.ts @@ -3,7 +3,7 @@ import {deprecate} from 'util' const pjson = require('../package.json') -import {APIClient} from './api-client' +import {APIClient, IOptions} from './api-client' import deps from './deps' const deprecatedCLI = deprecate(() => { @@ -17,7 +17,11 @@ export abstract class Command extends Base { get heroku(): APIClient { if (this._heroku) return this._heroku - this._heroku = new deps.APIClient(this.config) + const options: IOptions = { + debug: process.env.HEROKU_DEBUG === '1' || process.env.HEROKU_DEBUG?.toUpperCase() === 'TRUE', + debugHeaders: process.env.HEROKU_DEBUG_HEADERS === '1' || process.env.HEROKU_DEBUG_HEADERS?.toUpperCase() === 'TRUE', + } + this._heroku = new deps.APIClient(this.config, options) return this._heroku } diff --git a/test/api-client.test.ts b/test/api-client.test.ts index c83e26b..3214990 100644 --- a/test/api-client.test.ts +++ b/test/api-client.test.ts @@ -4,7 +4,6 @@ import base, {expect} from 'fancy-test' import nock from 'nock' import {resolve} from 'path' import * as sinon from 'sinon' - import {Command as CommandBase} from '../src/command' import {RequestId, requestIdHeader} from '../src/request-id' @@ -20,20 +19,22 @@ netrc.loadSync = function (this: typeof netrc) { } const env = process.env +const debug = require('debug') let api: nock.Scope -beforeEach(() => { - process.env = {} - api = nock('https://api.heroku.com') -}) -afterEach(() => { - process.env = env - api.done() -}) - -const test = base - .add('config', new Config({root: resolve(__dirname, '../package.json')})) +const test = base.add('config', new Config({root: resolve(__dirname, '../package.json')})) describe('api_client', () => { + beforeEach(function () { + process.env = {} + debug.disable('*') + api = nock('https://api.heroku.com') + }) + + afterEach(function () { + process.env = env + api.done() + }) + test .it('makes an HTTP request', async ctx => { api = nock('https://api.heroku.com', { @@ -44,7 +45,6 @@ describe('api_client', () => { const cmd = new Command([], ctx.config) const {body} = await cmd.heroku.get('/apps') expect(body).to.deep.equal([{name: 'myapp'}]) - // expect(netrc.loadSync).toBeCalled() }) test @@ -221,7 +221,7 @@ describe('api_client', () => { await cmd.heroku.get('/account') const stderrOutput = stderr.output.replace(/ *[»›] */g, '').replace(/ *\n */g, ' ') - expect(stderrOutput).to.include(`This account is delinquent with payment and we suspended it on ${suspensionTime}. If the account is still delinquent, we'll delete it on ${deletionTime}`) + expect(stderrOutput).to.include(`This account is delinquent with payment and we suspended it on ${suspensionTime}. If the account is still delinquent, we‘ll delete it on ${deletionTime}`) stderr.stop() particleboard.done() }) @@ -373,7 +373,7 @@ describe('api_client', () => { await cmd.heroku.get('/teams/my_team/members') const stderrOutput = stderr.output.replace(/ *[»›] */g, '').replace(/ *\n */g, ' ') - expect(stderrOutput).to.include(`This team is delinquent with payment and we suspended it on ${suspensionTime}. If the team is still delinquent, we'll delete it on ${deletionTime}`) + expect(stderrOutput).to.include(`This team is delinquent with payment and we suspended it on ${suspensionTime}. If the team is still delinquent, we‘ll delete it on ${deletionTime}`) stderr.stop() particleboard.done() }) @@ -455,6 +455,135 @@ describe('api_client', () => { expect((await dynos).body).to.deep.equal({web: 1}) }) + context('with HEROKU_DEBUG = "1"', function () { + context('without HEROKU_DEBUG_HEADERS = "1"', function () { + test + .it('enables only HTTP debug info', async ctx => { + process.env = { + HEROKU_DEBUG: '1', + } + api = nock('https://api.heroku.com', { + reqheaders: {authorization: 'Bearer mypass'}, + }) + api.get('/apps').reply(200, [{name: 'myapp'}]) + + const cmd = new Command([], ctx.config) + stderr.start() + await cmd.heroku.get('/apps') + stderr.stop() + + expect(cmd.heroku.options.debug).to.eq(true) + expect(cmd.heroku.options.debugHeaders).to.eq(false) + expect(stderr.output).to.contain('http → GET https://api.heroku.com/apps') + expect(stderr.output).not.to.contain("http accept: 'application/vnd.heroku+json; version=3") + }) + }) + + context('with HEROKU_DEBUG_HEADERS = "1"', function () { + test + .it('enables additional HTTP headers debug info', async ctx => { + process.env = { + HEROKU_DEBUG: '1', + HEROKU_DEBUG_HEADERS: '1', + } + api = nock('https://api.heroku.com', { + reqheaders: {authorization: 'Bearer mypass'}, + }) + api.get('/apps').reply(200, [{name: 'myapp'}]) + + const cmd = new Command([], ctx.config) + stderr.start() + await cmd.heroku.get('/apps') + stderr.stop() + + expect(cmd.heroku.options.debug).to.eq(true) + expect(cmd.heroku.options.debugHeaders).to.eq(true) + expect(stderr.output).to.contain('http → GET https://api.heroku.com/apps') + expect(stderr.output).to.contain("http accept: 'application/vnd.heroku+json; version=3") + }) + }) + }) + + context('without HEROKU_DEBUG = "1"', function () { + context('with HEROKU_DEBUG_HEADERS = "1"', function () { + test + .it('doesn‘t enable any HTTP debug info', async ctx => { + process.env = { + HEROKU_DEBUG_HEADERS: '1', + } + api = nock('https://api.heroku.com', { + reqheaders: {authorization: 'Bearer mypass'}, + }) + api.get('/apps').reply(200, [{name: 'myapp'}]) + + const cmd = new Command([], ctx.config) + stderr.start() + await cmd.heroku.get('/apps') + stderr.stop() + + expect(cmd.heroku.options.debug).to.eq(false) + expect(cmd.heroku.options.debugHeaders).to.eq(true) + expect(stderr.output).not.to.contain('http → GET https://api.heroku.com/apps') + expect(stderr.output).not.to.contain("http accept: 'application/vnd.heroku+json; version=3") + }) + }) + + context('without HEROKU_DEBUG_HEADERS = "1"', function () { + test + .it('doesn‘t enable any HTTP debug info', async ctx => { + api = nock('https://api.heroku.com', { + reqheaders: {authorization: 'Bearer mypass'}, + }) + api.get('/apps').reply(200, [{name: 'myapp'}]) + + const cmd = new Command([], ctx.config) + stderr.start() + await cmd.heroku.get('/apps') + stderr.stop() + + expect(cmd.heroku.options.debug).to.eq(false) + expect(cmd.heroku.options.debugHeaders).to.eq(false) + expect(stderr.output).not.to.contain('http → GET https://api.heroku.com/apps') + expect(stderr.output).not.to.contain("http accept: 'application/vnd.heroku+json; version=3") + }) + }) + }) + + context('with X-Heroku-Warning header set on response', function () { + test + .it('shows warnings', async ctx => { + api = nock('https://api.heroku.com', { + reqheaders: {authorization: 'Bearer mypass'}, + }) + api.get('/apps').reply(200, [], {'X-Heroku-Warning': ['warning message 1', 'warning message 2']}) + + const cmd = new Command([], ctx.config) + stderr.start() + await cmd.heroku.get('/apps') + stderr.stop() + + expect(stderr.output).to.contain('warning message 1') + expect(stderr.output).to.contain('warning message 2') + }) + }) + + context('with Warning-Message header set on response', function () { + test + .it('shows warnings', async ctx => { + api = nock('https://api.heroku.com', { + reqheaders: {authorization: 'Bearer mypass'}, + }) + api.get('/apps').reply(200, [], {'Warning-Message': 'warning message 1'}) + + const cmd = new Command([], ctx.config) + stderr.start() + await cmd.heroku.get('/apps') + stderr.stop() + + expect(stderr.output).to.contain('warning message 1') + }) + }) + context('request ids', function () { let generateStub: any