Improve robustness of request construction (#600)

* Improve robustness of request construction

* Fix flakey retry tests

Author: sholladay

package.json

@@ -22,7 +22,7 @@
 		"node": ">=18"
 	},
 	"scripts": {
-		"test": "xo && npm run build && ava --timeout=10m --serial",
+		"test": "xo && npm run build && ava",
 		"debug": "PWDEBUG=1 ava --timeout=2m",
 		"release": "np",
 		"build": "del-cli distribution && tsc --project tsconfig.dist.json",

source/core/Ky.ts

@@ -179,6 +179,11 @@ export class Ky {
 			this._options.duplex = 'half';
 		}
 
+		if (this._options.json !== undefined) {
+			this._options.body = this._options.stringifyJson?.(this._options.json) ?? JSON.stringify(this._options.json);
+			this._options.headers.set('content-type', this._options.headers.get('content-type') ?? 'application/json');
+		}
+
 		this.request = new globalThis.Request(this._input, this._options);
 
 		if (this._options.searchParams) {
@@ -201,12 +206,6 @@ export class Ky {
 			// The spread of `this.request` is required as otherwise it misses the `duplex` option for some reason and throws.
 			this.request = new globalThis.Request(new globalThis.Request(url, {...this.request}), this._options as RequestInit);
 		}
-
-		if (this._options.json !== undefined) {
-			this._options.body = this._options.stringifyJson?.(this._options.json) ?? JSON.stringify(this._options.json);
-			this.request.headers.set('content-type', this._options.headers.get('content-type') ?? 'application/json');
-			this.request = new globalThis.Request(this.request, {body: this._options.body});
-		}
 	}
 
 	protected _calculateRetryDelay(error: unknown) {

test/helpers/with-performance-observer.ts

@@ -0,0 +1,28 @@
+import {performance} from 'node:perf_hooks';
+import process from 'node:process';
+import type {ExecutionContext} from 'ava';
+
+type Argument = {
+	expectedDuration: number;
+	t: ExecutionContext;
+	test: () => Promise<void>;
+};
+
+// We allow the tests to take more time on CI than locally, to reduce flakiness
+const allowedOffset = process.env.CI ? 300 : 200;
+
+export async function withPerformance({
+	expectedDuration,
+	t,
+	test,
+}: Argument) {
+	const startTime = performance.now();
+	await test();
+	const endTime = performance.now();
+
+	const duration = endTime - startTime;
+	t.true(
+		Math.abs(duration - expectedDuration) < allowedOffset,
+		`Duration of ${duration}ms is not close to expected duration ${expectedDuration}ms`,
+	);
+}

test/helpers/with-performance.ts

@@ -1,7 +1,7 @@
 import test from 'ava';
 import ky from '../source/index.js';
 import {createHttpTestServer} from './helpers/create-http-test-server.js';
-import {withPerformanceObserver} from './helpers/with-performance-observer.js';
+import {withPerformance} from './helpers/with-performance.js';
 
 const fixture = 'fixture';
 const defaultRetryCount = 2;
@@ -442,7 +442,7 @@ test('throws when retry.statusCodes is not an array', async t => {
 	await server.close();
 });
 
-test('respect maximum backoff', async t => {
+test('respect maximum backoffLimit', async t => {
 	const retryCount = 4;
 	let requestCount = 0;
 
@@ -457,9 +457,8 @@ test('respect maximum backoff', async t => {
 		}
 	});
 
-	await withPerformanceObserver({
+	await withPerformance({
 		t,
-		name: 'default',
 		expectedDuration: 300 + 600 + 1200 + 2400,
 		async test() {
 			t.is(await ky(server.url, {
@@ -469,9 +468,9 @@ test('respect maximum backoff', async t => {
 	});
 
 	requestCount = 0;
-	await withPerformanceObserver({
+
+	await withPerformance({
 		t,
-		name: 'custom',
 		expectedDuration: 300 + 600 + 1000 + 1000,
 		async test() {
 			t.is(await ky(server.url, {
@@ -501,9 +500,8 @@ test('respect custom retry.delay', async t => {
 		}
 	});
 
-	await withPerformanceObserver({
+	await withPerformance({
 		t,
-		name: 'linear',
 		expectedDuration: 200 + 300 + 400 + 500,
 		async test() {
 			t.is(await ky(server.url, {