Skip to content

Commit

Permalink
fix(core): Handle text/* type fetch responses (#2456)
Browse files Browse the repository at this point in the history
* handle text responses

* update tests

* fix tests

* Replace error fallback with text/* special content handling

The outer `catch` was still responsible for handling errors,
so we can avoid duplication here and still use `statusText`.
We should instead add a separate case for `text/*` contents.
Those should never really be returned by endpoints, but if they
are we can return them as it was in this branch before.

* Update multipart snapshots

Co-authored-by: jdecroock <[email protected]>
  • Loading branch information
kitten and JoviDeCroock authored May 24, 2022
1 parent cabbcd7 commit 2695fb9
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 62 deletions.
5 changes: 5 additions & 0 deletions .changeset/moody-brooms-refuse.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@urql/core': patch
---

Passthrough responses with content type of `text/*` as error messages.
4 changes: 3 additions & 1 deletion exchanges/execute/src/execute.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,9 @@ describe('on operation', () => {

fetchMock.mockResolvedValue({
status: 200,
json: jest.fn().mockResolvedValue({ data: mockHttpResponseData }),
text: jest
.fn()
.mockResolvedValue(JSON.stringify({ data: mockHttpResponseData })),
});

const responseFromFetchExchange = await pipe(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`on error returns error data 1`] = `
Object {
"data": undefined,
"error": [CombinedError: [Network] ],
"error": [CombinedError: [Network] No Content],
"extensions": undefined,
"operation": Object {
"context": Object {
Expand Down Expand Up @@ -142,7 +142,7 @@ query getUser($name: String) { user(name: $name) { id firstName lastName } }",
exports[`on error returns error data with status 400 and manual redirect mode 1`] = `
Object {
"data": undefined,
"error": [CombinedError: [Network] ],
"error": [CombinedError: [Network] No Content],
"extensions": undefined,
"operation": Object {
"context": Object {
Expand Down
12 changes: 6 additions & 6 deletions exchanges/multipart-fetch/src/multipartFetchExchange.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ afterAll(() => {
(global as any).AbortController = undefined;
});

const response = {
const response = JSON.stringify({
status: 200,
data: {
data: {
user: 1200,
},
},
};
});

const exchangeArgs = {
forward: () => empty as Source<OperationResult>,
Expand All @@ -50,7 +50,7 @@ describe('on success', () => {
beforeEach(() => {
fetch.mockResolvedValue({
status: 200,
json: jest.fn().mockResolvedValue(response),
text: jest.fn().mockResolvedValue(response),
});
});

Expand Down Expand Up @@ -121,7 +121,7 @@ describe('on error', () => {
beforeEach(() => {
fetch.mockResolvedValue({
status: 400,
json: jest.fn().mockResolvedValue({}),
text: jest.fn().mockResolvedValue('{}'),
});
});

Expand Down Expand Up @@ -156,7 +156,7 @@ describe('on error', () => {
it('ignores the error when a result is available', async () => {
fetch.mockResolvedValue({
status: 400,
json: jest.fn().mockResolvedValue(response),
text: jest.fn().mockResolvedValue(response),
});

const data = await pipe(
Expand All @@ -165,7 +165,7 @@ describe('on error', () => {
toPromise
);

expect(data.data).toEqual(response.data);
expect(data.data).toEqual(JSON.parse(response).data);
});
});

Expand Down
62 changes: 31 additions & 31 deletions exchanges/persisted-fetch/src/persistedFetchExchange.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@ afterEach(() => {
});

it('accepts successful persisted query responses', async () => {
const expected = {
const expected = JSON.stringify({
data: {
test: true,
},
};
});

fetch.mockResolvedValueOnce({
json: () => Promise.resolve(expected),
text: () => Promise.resolve(expected),
});

const actual = await pipe(
Expand All @@ -50,22 +50,22 @@ it('accepts successful persisted query responses', async () => {
});

it('supports cache-miss persisted query errors', async () => {
const expectedMiss = {
const expectedMiss = JSON.stringify({
errors: [{ message: 'PersistedQueryNotFound' }],
};
});

const expectedRetry = {
const expectedRetry = JSON.stringify({
data: {
test: true,
},
};
});

fetch
.mockResolvedValueOnce({
json: () => Promise.resolve(expectedMiss),
text: () => Promise.resolve(expectedMiss),
})
.mockResolvedValueOnce({
json: () => Promise.resolve(expectedRetry),
text: () => Promise.resolve(expectedRetry),
});

const actual = await pipe(
Expand All @@ -81,22 +81,22 @@ it('supports cache-miss persisted query errors', async () => {
});

it('supports GET exclusively for persisted queries', async () => {
const expectedMiss = {
const expectedMiss = JSON.stringify({
errors: [{ message: 'PersistedQueryNotFound' }],
};
});

const expectedRetry = {
const expectedRetry = JSON.stringify({
data: {
test: true,
},
};
});

fetch
.mockResolvedValueOnce({
json: () => Promise.resolve(expectedMiss),
text: () => Promise.resolve(expectedMiss),
})
.mockResolvedValueOnce({
json: () => Promise.resolve(expectedRetry),
text: () => Promise.resolve(expectedRetry),
});

const actual = await pipe(
Expand All @@ -114,25 +114,25 @@ it('supports GET exclusively for persisted queries', async () => {
});

it('supports unsupported persisted query errors', async () => {
const expectedMiss = {
const expectedMiss = JSON.stringify({
errors: [{ message: 'PersistedQueryNotSupported' }],
};
});

const expectedRetry = {
const expectedRetry = JSON.stringify({
data: {
test: true,
},
};
});

fetch
.mockResolvedValueOnce({
json: () => Promise.resolve(expectedMiss),
text: () => Promise.resolve(expectedMiss),
})
.mockResolvedValueOnce({
json: () => Promise.resolve(expectedRetry),
text: () => Promise.resolve(expectedRetry),
})
.mockResolvedValueOnce({
json: () => Promise.resolve(expectedRetry),
text: () => Promise.resolve(expectedRetry),
});

const actual = await pipe(
Expand All @@ -148,14 +148,14 @@ it('supports unsupported persisted query errors', async () => {
});

it('correctly generates an SHA256 hash', async () => {
const expected = {
const expected = JSON.stringify({
data: {
test: true,
},
};
});

fetch.mockResolvedValue({
json: () => Promise.resolve(expected),
text: () => Promise.resolve(expected),
});

const queryHash = await hash(print(queryOperation.query));
Expand Down Expand Up @@ -185,14 +185,14 @@ it('correctly generates an SHA256 hash', async () => {
});

it('supports a custom hash function', async () => {
const expected = {
const expected = JSON.stringify({
data: {
test: true,
},
};
});

fetch.mockResolvedValueOnce({
json: () => Promise.resolve(expected),
text: () => Promise.resolve(expected),
});

const hashFn = jest.fn((_input: string, _doc: DocumentNode) => {
Expand Down Expand Up @@ -228,14 +228,14 @@ it('supports a custom hash function', async () => {
});

it('falls back to a non-persisted query if the hash is falsy', async () => {
const expected = {
const expected = JSON.stringify({
data: {
test: true,
},
};
});

fetch.mockResolvedValueOnce({
json: () => Promise.resolve(expected),
text: () => Promise.resolve(expected),
});

const hashFn = jest.fn(() => Promise.resolve(''));
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/exchanges/__snapshots__/fetch.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`on error returns error data 1`] = `
Object {
"data": undefined,
"error": [CombinedError: [Network] ],
"error": [CombinedError: [Network] No Content],
"extensions": undefined,
"operation": Object {
"context": Object {
Expand Down Expand Up @@ -142,7 +142,7 @@ query getUser($name: String) { user(name: $name) { id firstName lastName } }",
exports[`on error returns error data with status 400 and manual redirect mode 1`] = `
Object {
"data": undefined,
"error": [CombinedError: [Network] ],
"error": [CombinedError: [Network] No Content],
"extensions": undefined,
"operation": Object {
"context": Object {
Expand Down
12 changes: 6 additions & 6 deletions packages/core/src/exchanges/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@ afterAll(() => {
(global as any).AbortController = undefined;
});

const response = {
const response = JSON.stringify({
status: 200,
data: {
data: {
user: 1200,
},
},
};
});

const exchangeArgs = {
dispatchDebug: jest.fn(),
Expand All @@ -51,7 +51,7 @@ describe('on success', () => {
beforeEach(() => {
fetch.mockResolvedValue({
status: 200,
json: jest.fn().mockResolvedValue(response),
text: jest.fn().mockResolvedValue(response),
});
});

Expand Down Expand Up @@ -80,7 +80,7 @@ describe('on error', () => {
beforeEach(() => {
fetch.mockResolvedValue({
status: 400,
json: jest.fn().mockResolvedValue({}),
text: jest.fn().mockResolvedValue(JSON.stringify({})),
});
});

Expand Down Expand Up @@ -115,7 +115,7 @@ describe('on error', () => {
it('ignores the error when a result is available', async () => {
fetch.mockResolvedValue({
status: 400,
json: jest.fn().mockResolvedValue(response),
text: jest.fn().mockResolvedValue(response),
});

const data = await pipe(
Expand All @@ -124,7 +124,7 @@ describe('on error', () => {
toPromise
);

expect(data.data).toEqual(response.data);
expect(data.data).toEqual(JSON.parse(response).data);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`on error ignores the error when a result is available 1`] = `
Object {
"data": undefined,
"error": [CombinedError: [Network] ],
"error": [CombinedError: [Network] Forbidden],
"extensions": undefined,
"operation": Object {
"context": Object {
Expand Down Expand Up @@ -142,7 +142,7 @@ query getUser($name: String) { user(name: $name) { id firstName lastName } }",
exports[`on error returns error data 1`] = `
Object {
"data": undefined,
"error": [CombinedError: [Network] ],
"error": [CombinedError: [Network] Forbidden],
"extensions": undefined,
"operation": Object {
"context": Object {
Expand Down Expand Up @@ -281,7 +281,7 @@ query getUser($name: String) { user(name: $name) { id firstName lastName } }",
exports[`on error returns error data with status 400 and manual redirect mode 1`] = `
Object {
"data": undefined,
"error": [CombinedError: [Network] ],
"error": [CombinedError: [Network] Forbidden],
"extensions": undefined,
"operation": Object {
"context": Object {
Expand Down
Loading

0 comments on commit 2695fb9

Please sign in to comment.