diff --git a/package.json b/package.json index 878938c..8297bd5 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "scripts": { "start": "node src/index.js", "dev": "node --watch src/index.js", - "test": "node --test --import ./test/setup.js test/", + "test": "node --test --import ./test/setup.js 'test/**/*.test.js'", "lint": "eslint .", "lint:fix": "eslint . --fix", "format": "prettier --write .", diff --git a/src/http.js b/src/http.js index 40ad0bd..fe0979a 100644 --- a/src/http.js +++ b/src/http.js @@ -35,10 +35,16 @@ export async function requestJson(method, url, body, extra = {}) { 'Content-Type': 'application/json', ...(extra.headers ?? {}), }; + // `...extra` MUST come before `headers` so the merged headers win. + // Regression guard: putting `...extra` after `headers` (as this used + // to do) causes extra.headers to overwrite the merged object, + // stripping Content-Type — undici then defaults the body's + // Content-Type to text/plain;charset=UTF-8 and strict Webex + // endpoints (POST /locations, etc.) reply HTTP 415. const options = { method, - headers, ...extra, + headers, }; if (body !== undefined && body !== null) { options.body = typeof body === 'string' ? body : JSON.stringify(body); diff --git a/test/http.test.js b/test/http.test.js new file mode 100644 index 0000000..021f846 --- /dev/null +++ b/test/http.test.js @@ -0,0 +1,100 @@ +import { after, afterEach, before, beforeEach, describe, it } from 'node:test'; +import assert from 'node:assert/strict'; + +import { requestJson } from '../src/http.js'; + +// requestJson wraps global fetch, so we intercept there. Each test +// records the URL + options actually passed to fetch and returns a +// stubbed 200 JSON response. Restoring the real fetch after the suite +// keeps this file from leaking a mock into any later test file. +let originalFetch; +let calls; + +before(() => { + originalFetch = globalThis.fetch; +}); + +after(() => { + globalThis.fetch = originalFetch; +}); + +beforeEach(() => { + calls = []; + globalThis.fetch = async (url, options) => { + calls.push({ url, options }); + return new Response('{}', { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }; +}); + +afterEach(() => { + globalThis.fetch = originalFetch; +}); + +describe('requestJson', () => { + it('sends Content-Type: application/json on POSTs by default', async () => { + await requestJson('POST', 'https://example.test/x', { foo: 1 }); + assert.equal(calls[0].options.headers['Content-Type'], 'application/json'); + }); + + // Regression: prior to the fix, `options = { ...headers, ...extra }` + // let extra.headers overwrite the merged headers object, silently + // dropping Content-Type. Undici then guessed text/plain, which + // strict Webex endpoints (POST /locations, etc.) rejected with + // HTTP 415. This test locks the merge order down so any future + // regression fails loudly here rather than at 3am against Webex. + it('preserves Content-Type when the caller passes extra headers (e.g. Authorization)', async () => { + await requestJson( + 'POST', + 'https://example.test/x', + { foo: 1 }, + { + headers: { Authorization: 'Bearer test-token' }, + }, + ); + assert.equal(calls[0].options.headers['Content-Type'], 'application/json'); + assert.equal(calls[0].options.headers.Authorization, 'Bearer test-token'); + }); + + it('lets the caller override Content-Type when they explicitly need to', async () => { + await requestJson('POST', 'https://example.test/x', '', { + headers: { 'Content-Type': 'application/xml' }, + }); + assert.equal(calls[0].options.headers['Content-Type'], 'application/xml'); + }); + + it('JSON-stringifies plain-object bodies', async () => { + await requestJson('POST', 'https://example.test/x', { foo: 1, bar: [true] }); + assert.equal(calls[0].options.body, '{"foo":1,"bar":[true]}'); + }); + + it('passes string bodies through as-is (already serialized)', async () => { + await requestJson('POST', 'https://example.test/x', '{"already":"json"}'); + assert.equal(calls[0].options.body, '{"already":"json"}'); + }); + + it('sends no body when body is undefined or null', async () => { + await requestJson('GET', 'https://example.test/x'); + assert.equal(calls[0].options.body, undefined); + await requestJson('DELETE', 'https://example.test/x', null); + assert.equal(calls[1].options.body, undefined); + }); + + it('throws a descriptive error on non-2xx responses', async () => { + globalThis.fetch = async () => + new Response('{"message":"nope"}', { + status: 415, + statusText: 'Unsupported Media Type', + headers: { 'content-type': 'application/json' }, + }); + await assert.rejects(requestJson('POST', 'https://example.test/x', { foo: 1 }), (err) => { + assert.equal(err.status, 415); + assert.match(err.message, /HTTP 415/); + assert.match(err.message, /POST https:\/\/example\.test\/x/); + assert.equal(err.body, '{"message":"nope"}'); + return true; + }); + }); +});