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:password@a.com'),
+ ).rejects.toThrow(
+ new TypeError(
+ 'Request cannot be constructed from a URL that includes credentials: http://user:password@a.com/',
+ ),
+ );
+ });
+ 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 = {