Fix requestJson dropping Content-Type on POSTs (HTTP 415)
Some checks are pending
CI / verify (push) Waiting to run
Some checks are pending
CI / verify (push) Waiting to run
requestJson merged Content-Type: application/json into a headers object,
then spread `...extra` after `headers` in the fetch options — which let
extra.headers (always passed by webexJson for Authorization) overwrite
the merged object, stripping Content-Type entirely. Undici then
defaulted the string body's Content-Type to text/plain;charset=UTF-8,
and strict Webex endpoints like POST /locations rejected the request
with HTTP 415. Latent since the node-fetch -> native fetch migration.
Reorder to `{ method, ...extra, headers }` so the explicit merged
headers win, and add a test/http.test.js suite (7 tests) that pins the
merge behaviour down so this can't regress silently again.
Also: update the npm test script to `test/**/*.test.js` glob. Node
22.23 no longer accepts a bare `test/` directory as a positional arg
to `--test` (fails with ERR_UNSUPPORTED_DIR_IMPORT).
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
6b585bf33f
commit
203850fd69
3 changed files with 108 additions and 2 deletions
|
|
@ -13,7 +13,7 @@
|
||||||
"scripts": {
|
"scripts": {
|
||||||
"start": "node src/index.js",
|
"start": "node src/index.js",
|
||||||
"dev": "node --watch 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": "eslint .",
|
||||||
"lint:fix": "eslint . --fix",
|
"lint:fix": "eslint . --fix",
|
||||||
"format": "prettier --write .",
|
"format": "prettier --write .",
|
||||||
|
|
|
||||||
|
|
@ -35,10 +35,16 @@ export async function requestJson(method, url, body, extra = {}) {
|
||||||
'Content-Type': 'application/json',
|
'Content-Type': 'application/json',
|
||||||
...(extra.headers ?? {}),
|
...(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 = {
|
const options = {
|
||||||
method,
|
method,
|
||||||
headers,
|
|
||||||
...extra,
|
...extra,
|
||||||
|
headers,
|
||||||
};
|
};
|
||||||
if (body !== undefined && body !== null) {
|
if (body !== undefined && body !== null) {
|
||||||
options.body = typeof body === 'string' ? body : JSON.stringify(body);
|
options.body = typeof body === 'string' ? body : JSON.stringify(body);
|
||||||
|
|
|
||||||
100
test/http.test.js
Normal file
100
test/http.test.js
Normal file
|
|
@ -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', '<xml/>', {
|
||||||
|
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;
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
Loading…
Reference in a new issue