data and error both undefined for 500 status with an `errors` key in response

[maxsalven]

Describe the bug

My server returns status 400 and 500 errors with a JSON body {"errors":{"detail":"Bad Request"}}. If I execute a mutation that encounters a server exception, I'm seeing both data and error as undefined in the result:

So it seems URQL is somehow trying to parse {"errors":{"detail":"Bad Request"}} as a GQL response (which would be malformed as I think per the spec errors must be a non-empty list.). Unfortunately I'm not able to change that response shape. But I'd expect something in error still, given data is undefined. Is having both data and error as undefined after a mutation considered a bug?

If not, is there any way I can tell URQL that all 400 responses are network errors? I appreciate GQL is meant to be transport agnostic etc.

Reproduction

Change this line https://github.com/urql-graphql/urql/blob/052d5f491ccef808d78c42a2306d612f3984de33/packages/core/src/internal/fetchSource.test.ts#L105

to

text: vi.fn().mockResolvedValue('{"errors":{"detail":"Bad Request"}}'),

Any other key, e.g. "errorsFoo", and the tests pass.

Potential fix

There's probably a more reliable solution, but changing this line https://github.com/urql-graphql/urql/blob/052d5f491ccef808d78c42a2306d612f3984de33/packages/core/src/utils/result.ts#L31

to

  if (
    !('data' in result) &&
    (!('errors' in result) || !Array.isArray(result.errors))
  ) {

works

fetchSource.test.ts:

describe('on error with body', () => {
  beforeEach(() => {
    fetch.mockResolvedValue({
      status: 400,
      statusText: 'Forbidden',
      headers: { get: () => 'application/json' },
      text: vi.fn().mockResolvedValue('{"errors":{"detail":"Bad Request"}}'),
    });
  });

  it('handles network errors', async () => {
    const data = await pipe(
      makeFetchSource(queryOperation, 'https://test.com/graphql', {}),
      toPromise
    );

    expect(data).toMatchSnapshot();
    expect(data).toHaveProperty('error.networkError.message', 'Forbidden');
  });
});

Urql version

4.1.3

Validations

• [X] I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
• [X] Read the docs.
• [X] Follow our Code of Conduct

[JoviDeCroock]

Hey,

I see the problem yes, I would argue that errors (plural) evokes the expectation of the array and said expectation is also part of the spec 😅 I'm not sure whether it's worth adding the assertion that it's an array as it most likely does goof our types up a bit for i.e. the useQuery return value

[maxsalven]

As a mainstream example, Shopify will return 404 and 500 errors with a JSON payload that will cause URQL to not set result.error:

https://shopify.dev/docs/api/admin-graphql#status_and_error_codes

I appreciate it's not strictly per spec (as is using a non-200 status) but it's not an unrealistic scenario either.