Skip to content

Commit 8d2c2a1

Browse files
authored
refactor: getResponseData (#95)
2 parents 6c8b562 + 860d34e commit 8d2c2a1

File tree

7 files changed

+33
-34
lines changed

7 files changed

+33
-34
lines changed

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Releases
22

33
## Contents
4+
- [v0.3.4-alpha.0](#v0-3-4-alpha-0)
45
- [v0.3.3](#v0-3-3)
56
- [v0.3.2](#v0-3-2)
67
- [v0.3.1](#v0-3-1)
@@ -16,6 +17,12 @@
1617
- [v0.1.0](#v0-1-0)
1718
- [v0.0.8](#v0-0-8)
1819

20+
## v0.3.4-alpha.0
21+
22+
- Internal; Handle 205 status-codes more gracefully by defaulting to null returned as body.
23+
- Internal; Reduce overhead for getResponseData to read response once through `.text()` with attempting parsing on `application/json` or `+json` in the `Content-Type` header.
24+
25+
1926
## v0.3.3
2027

2128
- Bump @standard-schema/spec@1.1.0.

jsr.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@kasperrt/wiretyped",
3-
"version": "0.3.3",
3+
"version": "0.3.4-alpha.0",
44
"exports": {
55
".": "./src/index.ts"
66
},

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "wiretyped",
3-
"version": "0.3.3",
3+
"version": "0.3.4.alpha-0",
44
"description": "Universal fetch-based HTTP client with robust error handling, retries, caching, SSE, and Standard Schema validation.",
55
"type": "module",
66
"keywords": [

src/core/client.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -663,7 +663,9 @@ describe('RequestClient', () => {
663663
json: () => {
664664
throw new Error('test');
665665
},
666-
text: () => '{ "ok": true }',
666+
text: () => {
667+
throw new Error('test');
668+
},
667669
ok: true,
668670
status: 200,
669671
headers: { get: () => 'application/json' },
@@ -700,7 +702,7 @@ describe('RequestClient', () => {
700702
new Error('error doing request in get', {
701703
cause: new RetryExhaustedError('error retries exhausted', 10, {
702704
cause: new Error('error getting response in GET', {
703-
cause: new Error('error parsing json in getResponseData', { cause: new Error('test') }),
705+
cause: new Error('error reading response body in getResponseData', { cause: new Error('test') }),
704706
}),
705707
}),
706708
}),
@@ -1381,7 +1383,7 @@ describe('RequestClient', () => {
13811383
const getSpy = vi.spyOn(MOCK_FETCH_PROVIDER.prototype, 'get').mockImplementation(async () =>
13821384
asyncOk({
13831385
json: () => null,
1384-
text: () => '{ "data": "GET request data no params" }',
1386+
text: () => '{ "foo": "GET request data no params" }',
13851387
ok: true,
13861388
status: 200,
13871389
headers: { get: () => 'application/json' },

src/core/client.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,7 @@ export class RequestClient<Schema extends RequestDefinitions> {
428428
const close = (): void => controller.abort('closed by user request');
429429
const hasEventSchema = (name: string): name is SSEDataReturn<Schema, Endpoint & string>['type'] =>
430430
name in eventSchemas;
431+
431432
const parseBlock = async (block: string) => {
432433
if (!block) {
433434
return;

src/utils/getResponseData.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ describe('getResponseData', () => {
9191

9292
expect(value).toBeNull();
9393
expect(err).toBeInstanceOf(Error);
94-
expect(err?.message).toBe('error attempting string parse after json failed in getResponseData');
94+
expect(err?.message).toBe('error reading response body in getResponseData');
9595
expect(err?.cause).toBe(textError);
9696
});
9797

@@ -111,7 +111,7 @@ describe('getResponseData', () => {
111111

112112
expect(value).toBeNull();
113113
expect(err).toBeInstanceOf(Error);
114-
expect(err?.message).toBe('error json-parse string after json failed in getResponseData');
114+
expect(err?.message).toBe('error parsing json response body in getResponseData');
115115
expect(err?.cause).toBeInstanceOf(SyntaxError);
116116
});
117117

src/utils/getResponseData.ts

Lines changed: 16 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { isErrorType } from '../error/isErrorType.js';
21
import type { FetchResponse } from '../types/request.js';
32
import { type SafeWrapAsync, safeWrap, safeWrapAsync } from './wrap.js';
43

@@ -22,41 +21,31 @@ import { type SafeWrapAsync, safeWrap, safeWrapAsync } from './wrap.js';
2221
* and `value` is the parsed body (`ReturnValue | null`).
2322
*/
2423
export async function getResponseData<ReturnValue>(response: FetchResponse): SafeWrapAsync<Error, ReturnValue> {
25-
if (response.status === 204) {
24+
// Per HTTP spec, 204 + 205 shouldn't have a body
25+
if (response.status === 204 || response.status === 205) {
2626
return [null, null as ReturnValue];
2727
}
2828

29-
const contentType = response.headers.get('Content-Type');
30-
if (!contentType?.includes('application/json')) {
31-
const [errText, text] = await safeWrapAsync(() => response.text());
32-
if (errText) {
33-
return [new Error('error parsing text in getResponseData', { cause: errText }), null];
34-
}
35-
36-
return [null, (text || null) as ReturnValue];
37-
}
38-
39-
const [errJson, json] = await safeWrapAsync(() => response.json());
40-
// if we get a normal non TypeError, we want to try more aggressively at parsing this still, in case
41-
// the browser fumbled and didn't supply us with a proper json method
42-
if (errJson && !isErrorType(TypeError, errJson)) {
43-
return [new Error('error parsing json in getResponseData', { cause: errJson }), null];
29+
// Use .text as reader, since double reads with text -> json would cause TypeError
30+
// due to the body being consumed already
31+
const [errText, text] = await safeWrapAsync(() => response.text());
32+
if (errText) {
33+
return [new Error('error reading response body in getResponseData', { cause: errText }), null];
4434
}
4535

46-
if (!isErrorType(TypeError, errJson)) {
47-
return [null, json];
36+
if (!text) {
37+
return [null, null as ReturnValue];
4838
}
4939

50-
// If TypeError, try again, more aggressively
51-
const [errText, text] = await safeWrapAsync(() => response.text());
52-
if (errText) {
53-
return [new Error('error attempting string parse after json failed in getResponseData', { cause: errText }), null];
40+
const contentType = response?.headers?.get('Content-Type')?.toLowerCase();
41+
if (!contentType?.includes('application/json') && !contentType?.includes('+json')) {
42+
return [null, text as ReturnValue];
5443
}
5544

56-
const [errParse, parsed] = safeWrap<Error, ReturnValue>(() => JSON.parse(text));
57-
if (errParse) {
58-
return [new Error('error json-parse string after json failed in getResponseData', { cause: errParse }), null];
45+
const [errJson, json] = safeWrap(() => JSON.parse(text));
46+
if (errJson) {
47+
return [new Error('error parsing json response body in getResponseData', { cause: errJson }), null];
5948
}
6049

61-
return [null, parsed];
50+
return [null, json];
6251
}

0 commit comments

Comments
 (0)