fix(fetch): handle non object bodies properly (#786)

packages/fetch/README.md

@@ -48,9 +48,31 @@ npm install @sapphire/fetch
 
 ## Usage
 
-**Note:** While this section uses `import`, it maps 1:1 with CommonJS' require syntax. For example, `import { fetch } from '@sapphire/fetch'` is the same as `const { fetch } = require('@sapphire/fetch')`.
-
-**Note**: `fetch` can also be imported as a default import: `import fetch from '@sapphire/fetch'`.
+> [!NOTE]
+> While this section uses `import`, it maps 1:1 with CommonJS' require syntax. For example,
+>
+> ```ts
+> import { fetch } from '@sapphire/fetch';
+> ```
+>
+> is the same as
+>
+> ```ts
+> const { fetch } = require('@sapphire/fetch');
+> ```
+
+> [!IMPORTANT]
+> When providing a serializable object to the `body` option, `@sapphire/fetch` will automatically call `JSON.stringify` on the object. This means you can pass an object directly to the `body` option without having to call `JSON.stringify` yourself.
+> If the body is _not_ serializable (such as a `File`, `Buffer`, or `Blob`), the body will be sent as-is.
+> Serializability is calculated based on:
+>
+> -   If the body is `null`
+> -   If the body's `.constructor` property is `undefined`
+> -   If the body's `.constructor.name` property is `Object`
+> -   If the body has a function property named `toJSON`
+
+> [!WARNING]
+> Because `@sapphire/fetch` aims to be as close to global fetch as possible, it doesn't support proxy options that a library like undici does. If you want to use a proxy, you should use undici directly.
 
 ### `GET`ting JSON data
 
@@ -99,9 +121,7 @@ const responseData = await fetch(
 		headers: {
 			'Content-Type': 'application/json'
 		},
-		body: JSON.stringify({
-			name: 'John Doe'
-		})
+		body: { name: 'John Doe' }
 	},
 	FetchResultTypes.JSON
 );

packages/fetch/src/lib/fetch.ts

@@ -153,14 +153,17 @@ export async function fetch(url: URL | string, options?: RequestOptions | FetchR
 
 	let { body } = options;
 
-	if (body && typeof body === 'object') {
+	if (shouldJsonStringify(body)) {
 		body = JSON.stringify(body);
 	}
 
 	// Transform the URL to a String, in case an URL object was passed
 	const stringUrl = String(url);
 
-	const result: Response = await globalThis.fetch(stringUrl, { ...options, body });
+	const result: Response = await globalThis.fetch(stringUrl, {
+		...options,
+		body: body as RequestInit['body']
+	});
 	if (!result.ok) throw new QueryError(stringUrl, result.status, result, await result.clone().text());
 
 	switch (type) {
@@ -178,3 +181,28 @@ export async function fetch(url: URL | string, options?: RequestOptions | FetchR
 			throw new Error(`Unknown type "${type}"`);
 	}
 }
+
+/**
+ * Determines whether a value should be stringified as JSON.
+ *
+ * @param value - The value to check.
+ * @returns A boolean indicating whether the value should be stringified as JSON.
+ */
+function shouldJsonStringify(value: unknown) {
+	// If the value is not an object, it should not be stringified
+	if (typeof value !== 'object') return false;
+	// Buffers should not be stringified
+	if (typeof Buffer !== 'undefined' && Buffer.isBuffer(value)) return false;
+
+	// null object
+	if (value === null) return true;
+	// Object.create(null)
+	if (value.constructor === undefined) return true;
+	// Plain objects
+	if (value.constructor === Object) return true;
+	// Has toJSON method
+	if ('toJSON' in value && typeof value.toJSON === 'function') return true;
+
+	// Anything else (such as streams or unserializables)
+	return false;
+}

packages/fetch/src/lib/types.ts

@@ -267,5 +267,5 @@ export enum FetchMediaContentTypes {
 }
 
 export interface RequestOptions extends Omit<RequestInit, 'body'> {
-	body?: BodyInit | Record<any, any>;
+	body?: RequestInit['body'] | Record<any, any>;
 }

packages/fetch/tests/fetch.test.ts

@@ -14,7 +14,15 @@ describe('fetch', () => {
 
 			return HttpResponse.json({ test: false }, { status: 400 });
 		}),
-		http.get('http://localhost/404', () => HttpResponse.json({ success: false }, { status: 404 }))
+		http.get('http://localhost/404', () => HttpResponse.json({ success: false }, { status: 404 })),
+		http.post('http://localhost/upload', async ({ request }) => {
+			try {
+				await request.json();
+				return HttpResponse.json({ message: 'Successfully parsed body as JSON, this is unexpected!!' }, { status: 200 });
+			} catch (error) {
+				return HttpResponse.json({ message: 'Failed to parse body as JSON, this is expected!!' }, { status: 200 });
+			}
+		})
 	);
 
 	beforeAll(() => server.listen());
@@ -101,6 +109,34 @@ describe('fetch', () => {
 
 			expect(response.test).toBe(true);
 		});
+
+		test('GIVEN fetch w/ Blob body THEN returns successfully', async () => {
+			const response = await fetch<{ message: string }>(
+				'http://localhost/upload',
+				{
+					method: FetchMethods.Post,
+					body: new Blob(['De Blob'], {
+						type: FetchMediaContentTypes.TextPlain
+					})
+				},
+				FetchResultTypes.JSON
+			);
+
+			expect(response.message).toBe('Failed to parse body as JSON, this is expected!!');
+		});
+
+		test('GIVEN fetch w/ buffer body THEN returns successfully', async () => {
+			const response = await fetch<{ message: string }>(
+				'http://localhost/upload',
+				{
+					method: FetchMethods.Post,
+					body: Buffer.alloc(1)
+				},
+				FetchResultTypes.JSON
+			);
+
+			expect(response.message).toBe('Failed to parse body as JSON, this is expected!!');
+		});
 	});
 
 	describe('Unsuccessful fetches', () => {