Merge pull request #761 from wheresrhys/rhys/error-handling

Rhys/error handling

packages/core/src/RequestUtils.js

@@ -44,12 +44,19 @@ export function createCallLogFromUrlAndOptions(url, options) {
 	if (typeof url === 'string' || url instanceof String || url instanceof URL) {
 		// @ts-ignore - jsdoc doesn't distinguish between string and String, but typechecker complains
 		url = normalizeUrl(url);
+		const derivedOptions = options ? { ...options } : {};
+		if (derivedOptions.headers) {
+			derivedOptions.headers = normalizeHeaders(derivedOptions.headers);
+		}
+		derivedOptions.method = derivedOptions.method
+			? derivedOptions.method.toLowerCase()
+			: 'get';
 		return {
 			args: [url, options],
 			url,
 			queryParams: new URLSearchParams(getQuery(url)),
-			options: options || {},
-			signal: options && options.signal,
+			options: derivedOptions,
+			signal: derivedOptions.signal,
 			pendingPromises,
 		};
 	}
@@ -120,14 +127,18 @@ export function getQuery(url) {
 
 /**
  *
- * @param {Headers | [string, string][] | Record < string, string > | Object.<string, string | number> | HeadersInit} headers
+ * @param {HeadersInit | Object.<string, string |number>} headers
  * @returns {Object.<string, string>}
  */
 export const normalizeHeaders = (headers) => {
-	const entries =
-		headers instanceof Headers
-			? [...headers.entries()]
-			: Object.entries(headers);
+	let entries;
+	if (headers instanceof Headers) {
+		entries = [...headers.entries()];
+	} else if (Array.isArray(headers)) {
+		entries = headers;
+	} else {
+		entries = Object.entries(headers);
+	}
 	return Object.fromEntries(
 		entries.map(([key, val]) => [key.toLowerCase(), String(val).valueOf()]),
 	);

packages/core/src/Router.js

@@ -93,6 +93,30 @@ function shouldSendAsObject(responseInput) {
 	return true;
 }
 
+/**
+ *
+ * @param {CallLog} callLog
+ */
+function throwSpecExceptions({ url, options: { headers, method, body } }) {
+	if (headers) {
+		Object.entries(headers).forEach(([key]) => {
+			if (/\s/.test(key)) {
+				throw new TypeError('Invalid name');
+			}
+		});
+	}
+	const urlObject = new URL(url);
+	if (urlObject.username || urlObject.password) {
+		throw new TypeError(
+			`Request cannot be constructed from a URL that includes credentials: ${url}`,
+		);
+	}
+
+	if (['get', 'head'].includes(method) && body) {
+		throw new TypeError('Request with GET/HEAD method cannot have body.');
+	}
+}
+
 /**
  * @param {CallLog} callLog
  * @returns
@@ -149,6 +173,7 @@ export default class Router {
 	 * @returns {Promise<Response>}
 	 */
 	execute(callLog) {
+		throwSpecExceptions(callLog);
 		// TODO make abort vs reject neater
 		return new Promise(async (resolve, reject) => {
 			const { url, options, request, pendingPromises } = callLog;

packages/core/src/__tests__/CallHistory.test.js

@@ -9,7 +9,7 @@ describe('CallHistory', () => {
 	});
 
 	const fetchTheseUrls = (...urls) =>
-		Promise.all(urls.map(fm.fetchHandler.bind(fm)));
+		Promise.all(urls.map((url) => fm.fetchHandler(url)));
 
 	describe('helper methods', () => {
 		describe('called()', () => {
@@ -208,8 +208,8 @@ describe('CallHistory', () => {
 				describe('boolean and named route filters', () => {
 					it('can retrieve calls matched by non-fallback routes', async () => {
 						fm.route('http://a.com/', 200).catch();
-
 						await fetchTheseUrls('http://a.com/', 'http://b.com/');
+
 						expectSingleUrl(true)('http://a.com/');
 						expectSingleUrl('matched')('http://a.com/');
 					});
@@ -298,12 +298,13 @@ describe('CallHistory', () => {
 							headers: { a: 'z' },
 						});
 						expect(filteredCalls.length).toEqual(1);
+
 						expect(filteredCalls[0]).toMatchObject(
 							expect.objectContaining({
 								url: 'http://a.com/',
-								options: {
+								options: expect.objectContaining({
 									headers: { a: 'z' },
-								},
+								}),
 							}),
 						);
 					});
@@ -326,9 +327,9 @@ describe('CallHistory', () => {
 						expect(filteredCalls[0]).toMatchObject(
 							expect.objectContaining({
 								url: 'http://b.com/',
-								options: {
+								options: expect.objectContaining({
 									headers: { a: 'z' },
-								},
+								}),
 							}),
 						);
 					});
@@ -347,9 +348,9 @@ describe('CallHistory', () => {
 						expect(filteredCalls[0]).toMatchObject(
 							expect.objectContaining({
 								url: 'http://a.com/',
-								options: {
+								options: expect.objectContaining({
 									headers: { a: 'z' },
-								},
+								}),
 							}),
 						);
 					});

packages/core/src/__tests__/Matchers/header.js → packages/core/src/__tests__/Matchers/headers.test.js

@@ -5,11 +5,10 @@ describe('header matching', () => {
 	it('not match when headers not present', () => {
 		const route = new Route({
 			headers: { a: 'b' },
-
 			response: 200,
 		});
 
-		expect(route.matcher({ url: 'http://a.com/' })).toBe(true);
+		expect(route.matcher({ url: 'http://a.com/', options: {} })).toBe(false);
 	});
 
 	it("not match when headers don't match", () => {
@@ -154,7 +153,9 @@ describe('header matching', () => {
 			headers: { a: 'b' },
 		});
 
-		expect(route.matcher({ url: 'http://domain.com/person' })).toBe(false);
+		expect(
+			route.matcher({ url: 'http://domain.com/person', options: {} }),
+		).toBe(false);
 		expect(
 			route.matcher({
 				url: 'http://domain.com/person',
@@ -165,33 +166,30 @@ describe('header matching', () => {
 		).toBe(true);
 	});
 
-	it('match custom Headers instance', () => {
-		const MyHeaders = class {
-			constructor(obj) {
-				this.obj = obj;
-			}
-			// eslint-disable-next-line class-methods-use-this
-			*[Symbol.iterator]() {
-				yield ['a', 'b'];
-			}
-			// eslint-disable-next-line class-methods-use-this
-			has() {
-				return true;
-			}
-		};
-
+	it('can match against a Headers instance', () => {
 		const route = new Route({
+			headers: { a: 'b' },
 			response: 200,
+		});
+		const headers = new Headers();
+
+		headers.append('a', 'b');
+
+		expect(route.matcher({ url: 'http://a.com/', options: { headers } })).toBe(
+			true,
+		);
+	});
+
+	it('can match against an array of arrays', () => {
+		const route = new Route({
 			headers: { a: 'b' },
-			config: { Headers: MyHeaders },
+			response: 200,
 		});
 
 		expect(
 			route.matcher({
-				url: 'http://a.com',
-				options: {
-					headers: new MyHeaders({ a: 'b' }),
-				},
+				url: 'http://a.com/',
+				options: { headers: [['a', 'b']] },
 			}),
 		).toBe(true);
 	});

packages/core/src/__tests__/spec-compliance.test.js

@@ -0,0 +1,44 @@
+import { describe, expect, it, beforeAll } from 'vitest';
+import fetchMock from '../FetchMock';
+describe('Spec compliance', () => {
+	// NOTE: these are not exhaustive, but feel like a sensible, reasonably easy to implement subset
+	// https://developer.mozilla.org/en-US/docs/Web/API/Window/fetch#exceptions
+	describe('exceptions', () => {
+		beforeAll(() => fetchMock.catch());
+		it('reject on invalid header name', async () => {
+			await expect(
+				fetchMock.fetchHandler('http://a.com', {
+					headers: {
+						'has space': 'ok',
+					},
+				}),
+			).rejects.toThrow(new TypeError('Invalid name'));
+		});
+		it('reject on url containing credentials', async () => {
+			await expect(
+				fetchMock.fetchHandler('http://user:[email protected]'),
+			).rejects.toThrow(
+				new TypeError(
+					'Request cannot be constructed from a URL that includes credentials: http://user:[email protected]/',
+				),
+			);
+		});
+		it('reject if the request method is GET or HEAD and the body is non-null.', async () => {
+			await expect(
+				fetchMock.fetchHandler('http://a.com', { body: 'a' }),
+			).rejects.toThrow(
+				new TypeError('Request with GET/HEAD method cannot have body.'),
+			);
+			await expect(
+				fetchMock.fetchHandler('http://a.com', { body: 'a', method: 'GET' }),
+			).rejects.toThrow(
+				new TypeError('Request with GET/HEAD method cannot have body.'),
+			);
+			await expect(
+				fetchMock.fetchHandler('http://a.com', { body: 'a', method: 'HEAD' }),
+			).rejects.toThrow(
+				new TypeError('Request with GET/HEAD method cannot have body.'),
+			);
+		});
+	});
+});

packages/core/types/RequestUtils.d.ts

@@ -3,9 +3,9 @@ export function createCallLogFromUrlAndOptions(url: string | object, options: Re
 export function createCallLogFromRequest(request: Request, options: RequestInit): Promise<CallLog>;
 export function getPath(url: string): string;
 export function getQuery(url: string): string;
-export function normalizeHeaders(headers: Headers | [string, string][] | Record<string, string> | {
+export function normalizeHeaders(headers: HeadersInit | {
     [x: string]: string | number;
-} | HeadersInit): {
+}): {
     [x: string]: string;
 };
 export type DerivedRequestOptions = {