From f84d1846bca883e5e2127315565de4f92f26a294 Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Tue, 20 Dec 2022 13:53:59 -0800 Subject: [PATCH 1/3] feat(api): surface `Warning` response headers --- __tests__/lib/fetch.test.ts | 74 +++++++++++++++++++++++++++++++++++++ src/lib/fetch.ts | 58 ++++++++++++++++++++++++++++- src/lib/logger.ts | 6 ++- 3 files changed, 135 insertions(+), 3 deletions(-) diff --git a/__tests__/lib/fetch.test.ts b/__tests__/lib/fetch.test.ts index 633ff1dfa..4ac59c787 100644 --- a/__tests__/lib/fetch.test.ts +++ b/__tests__/lib/fetch.test.ts @@ -106,6 +106,80 @@ describe('#fetch()', () => { mock.done(); }); + describe('warning response header', () => { + let consoleWarnSpy; + + const getWarningCommandOutput = () => { + return [consoleWarnSpy.mock.calls.join('\n\n')].filter(Boolean).join('\n\n'); + }; + + beforeEach(() => { + consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(); + }); + + afterEach(() => { + consoleWarnSpy.mockRestore(); + }); + + it('should not log anything if no warning header was passed', async () => { + const mock = getAPIMock().get('/api/v1/some-warning').reply(200, undefined, { + Warning: '', + }); + + await fetch(`${config.get('host')}/api/v1/some-warning`); + + // eslint-disable-next-line no-console + expect(console.warn).toHaveBeenCalledTimes(0); + expect(getWarningCommandOutput()).toBe(''); + + mock.done(); + }); + + it('should surface a single warning header', async () => { + const mock = getAPIMock().get('/api/v1/some-warning').reply(200, undefined, { + Warning: '199 - "some error"', + }); + + await fetch(`${config.get('host')}/api/v1/some-warning`); + + // eslint-disable-next-line no-console + expect(console.warn).toHaveBeenCalledTimes(1); + expect(getWarningCommandOutput()).toBe('⚠️ ReadMe API Warning: some error'); + + mock.done(); + }); + + it('should surface multiple warning headers', async () => { + const mock = getAPIMock().get('/api/v1/some-warning').reply(200, undefined, { + Warning: '199 - "some error" 199 - "another error"', + }); + + await fetch(`${config.get('host')}/api/v1/some-warning`); + + // eslint-disable-next-line no-console + expect(console.warn).toHaveBeenCalledTimes(2); + expect(getWarningCommandOutput()).toBe( + '⚠️ ReadMe API Warning: some error\n\n⚠️ ReadMe API Warning: another error' + ); + + mock.done(); + }); + + it('should surface header content even if parsing fails', async () => { + const mock = getAPIMock().get('/api/v1/some-warning').reply(200, undefined, { + Warning: 'some garbage error', + }); + + await fetch(`${config.get('host')}/api/v1/some-warning`); + + // eslint-disable-next-line no-console + expect(console.warn).toHaveBeenCalledTimes(1); + expect(getWarningCommandOutput()).toBe('⚠️ ReadMe API Warning: some garbage error'); + + mock.done(); + }); + }); + describe('proxies', () => { afterEach(() => { delete process.env.https_proxy; diff --git a/src/lib/fetch.ts b/src/lib/fetch.ts index 9f5c2702d..3c53fa087 100644 --- a/src/lib/fetch.ts +++ b/src/lib/fetch.ts @@ -8,7 +8,7 @@ import pkg from '../../package.json'; import APIError from './apiError'; import { isGHA } from './isCI'; -import { debug } from './logger'; +import { debug, warn } from './logger'; const SUCCESS_NO_CONTENT = 204; @@ -22,6 +22,52 @@ function getProxy() { return ''; } +interface WarningHeaderObject { + code: string; + agent: string; + message: string; + date?: string; +} + +function stripQuotes(s: string) { + if (!s) return ''; + return s.replace(/(^"|[",]*$)/g, ''); +} + +/** + * Parses Warning header into actionable object + * @param header raw `Warning` header + * @see {@link https://www.rfc-editor.org/rfc/rfc7234#section-5.5} + * @see {@link https://github.com/marcbachmann/warning-header-parser} + */ +function parseWarningHeader(header: string): WarningHeaderObject[] { + try { + const warnings = header.split(/([0-9]{3} [a-z0-9.@\-/]*) /g); + + let previous: WarningHeaderObject; + + return warnings.reduce((all, w) => { + // eslint-disable-next-line no-param-reassign + w = w.trim(); + const newError = w.match(/^([0-9]{3}) (.*)/); + if (newError) { + previous = { code: newError[1], agent: newError[2], message: '' }; + } else if (w) { + const errorContent = w.split(/" "/); + if (errorContent) { + previous.message = stripQuotes(errorContent[0]); + previous.date = stripQuotes(errorContent[1]); + all.push(previous); + } + } + return all; + }, []); + } catch (e) { + debug(`error parsing warning header: ${e.message}`); + return [{ code: '199', agent: '-', message: header }]; + } +} + /** * Getter function for a string to be used in the user-agent header based on the current * environment. @@ -64,6 +110,16 @@ export default function fetch(url: string, options: RequestInit = { headers: new return nodeFetch(fullUrl, { ...options, headers, + }).then(res => { + const warningHeader = res.headers.get('Warning'); + if (warningHeader) { + debug(`received warning header: ${warningHeader}`); + const warnings = parseWarningHeader(warningHeader); + warnings.forEach(warning => { + warn(warning.message, 'ReadMe API Warning:'); + }); + } + return res; }); } diff --git a/src/lib/logger.ts b/src/lib/logger.ts index 13d496f50..963b33f54 100644 --- a/src/lib/logger.ts +++ b/src/lib/logger.ts @@ -68,12 +68,14 @@ function oraOptions() { /** * Wrapper for warn statements. + * @param prefix Text that precedes the warning. + * This is *not* used in the GitHub Actions-formatted warning. */ -function warn(input: string) { +function warn(input: string, prefix = 'Warning!') { /* istanbul ignore next */ if (isGHA() && !isTest()) return core.warning(input); // eslint-disable-next-line no-console - return console.warn(chalk.yellow(`⚠️ Warning! ${input}`)); + return console.warn(chalk.yellow(`⚠️ ${prefix} ${input}`)); } export { debug, error, info, oraOptions, warn }; From bc314b212c5b64582504bde04ae9137c24448e68 Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Tue, 20 Dec 2022 13:58:50 -0800 Subject: [PATCH 2/3] chore: var names and docs --- src/lib/fetch.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/lib/fetch.ts b/src/lib/fetch.ts index 3c53fa087..6bf0ccd65 100644 --- a/src/lib/fetch.ts +++ b/src/lib/fetch.ts @@ -22,7 +22,11 @@ function getProxy() { return ''; } -interface WarningHeaderObject { +/** + * @see {@link https://www.rfc-editor.org/rfc/rfc7234#section-5.5} + * @see {@link https://github.com/marcbachmann/warning-header-parser} + */ +interface WarningHeader { code: string; agent: string; message: string; @@ -35,16 +39,16 @@ function stripQuotes(s: string) { } /** - * Parses Warning header into actionable object + * Parses Warning header into an array of warning header objects * @param header raw `Warning` header * @see {@link https://www.rfc-editor.org/rfc/rfc7234#section-5.5} * @see {@link https://github.com/marcbachmann/warning-header-parser} */ -function parseWarningHeader(header: string): WarningHeaderObject[] { +function parseWarningHeader(header: string): WarningHeader[] { try { const warnings = header.split(/([0-9]{3} [a-z0-9.@\-/]*) /g); - let previous: WarningHeaderObject; + let previous: WarningHeader; return warnings.reduce((all, w) => { // eslint-disable-next-line no-param-reassign From b7c1a33e95ce24e447ab1851b9efeed771331c96 Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Tue, 20 Dec 2022 14:02:35 -0800 Subject: [PATCH 3/3] docs: add MDN link --- src/lib/fetch.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lib/fetch.ts b/src/lib/fetch.ts index 6bf0ccd65..a95707771 100644 --- a/src/lib/fetch.ts +++ b/src/lib/fetch.ts @@ -23,6 +23,7 @@ function getProxy() { } /** + * @see {@link https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Warning} * @see {@link https://www.rfc-editor.org/rfc/rfc7234#section-5.5} * @see {@link https://github.com/marcbachmann/warning-header-parser} */ @@ -41,6 +42,7 @@ function stripQuotes(s: string) { /** * Parses Warning header into an array of warning header objects * @param header raw `Warning` header + * @see {@link https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Warning} * @see {@link https://www.rfc-editor.org/rfc/rfc7234#section-5.5} * @see {@link https://github.com/marcbachmann/warning-header-parser} */