From 5ea5675fc922ec06f37afc4ba7cf69a70a0bd1f5 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Tue, 21 Oct 2025 21:34:54 -0400 Subject: [PATCH 1/5] Respect maxRetries for unexpected errors --- packages/toolkit/src/query/retry.ts | 26 ++-- .../toolkit/src/query/tests/retry.test.ts | 136 ++++++++++++++++++ 2 files changed, 153 insertions(+), 9 deletions(-) diff --git a/packages/toolkit/src/query/retry.ts b/packages/toolkit/src/query/retry.ts index 3947dd4acd..cfee5b1e1e 100644 --- a/packages/toolkit/src/query/retry.ts +++ b/packages/toolkit/src/query/retry.ts @@ -127,16 +127,24 @@ const retryWithBackoff: BaseQueryEnhancer< throw e } - if ( - e instanceof HandledError && - !options.retryCondition(e.value.error as FetchBaseQueryError, args, { - attempt: retry, - baseQueryApi: api, - extraOptions, - }) - ) { - return e.value + if (e instanceof HandledError) { + if ( + !options.retryCondition(e.value.error as FetchBaseQueryError, args, { + attempt: retry, + baseQueryApi: api, + extraOptions, + }) + ) { + return e.value // Max retries for expected error + } + } else { + // For unexpected errors, respect maxRetries + if (retry > options.maxRetries) { + // Return the error as a proper error response instead of throwing + return { error: e } + } } + await options.backoff(retry, options.maxRetries) } } diff --git a/packages/toolkit/src/query/tests/retry.test.ts b/packages/toolkit/src/query/tests/retry.test.ts index b2233021b0..f91d547309 100644 --- a/packages/toolkit/src/query/tests/retry.test.ts +++ b/packages/toolkit/src/query/tests/retry.test.ts @@ -548,4 +548,140 @@ describe('configuration', () => { expect(baseBaseQuery).toHaveBeenCalled() expect(baseBaseQuery.mock.calls.length).toBeLessThan(10) }) + + // Tests for issue #4079: Thrown errors should respect maxRetries + test('thrown errors (not HandledError) should respect maxRetries', async () => { + const baseBaseQuery = vi.fn< + Parameters, + ReturnType + >() + // Simulate network error that keeps throwing + baseBaseQuery.mockRejectedValue(new Error('Network timeout')) + + const baseQuery = retry(baseBaseQuery, { maxRetries: 3 }) + const api = createApi({ + baseQuery, + endpoints: (build) => ({ + q1: build.query({ + query: () => {}, + }), + }), + }) + + const storeRef = setupApiStore(api, undefined, { + withoutTestLifecycles: true, + }) + + storeRef.store.dispatch(api.endpoints.q1.initiate({})) + + await loopTimers(5) + + // Should try initial + 3 retries = 4 total, then stop + // Currently this will fail because it retries infinitely + expect(baseBaseQuery).toHaveBeenCalledTimes(4) + }) + + test('graphql-style thrown errors should respect maxRetries', async () => { + class ClientError extends Error { + constructor(message: string) { + super(message) + this.name = 'ClientError' + } + } + + const baseBaseQuery = vi.fn< + Parameters, + ReturnType + >() + // Simulate graphql-request throwing ClientError + baseBaseQuery.mockImplementation(() => { + throw new ClientError('GraphQL network error') + }) + + const baseQuery = retry(baseBaseQuery, { maxRetries: 2 }) + const api = createApi({ + baseQuery, + endpoints: (build) => ({ + q1: build.query({ + query: () => {}, + }), + }), + }) + + const storeRef = setupApiStore(api, undefined, { + withoutTestLifecycles: true, + }) + + storeRef.store.dispatch(api.endpoints.q1.initiate({})) + + await loopTimers(4) + + // Should try initial + 2 retries = 3 total, then stop + // Currently this will fail because it retries infinitely + expect(baseBaseQuery).toHaveBeenCalledTimes(3) + }) + + test('handles mix of returned errors and thrown errors', async () => { + const baseBaseQuery = vi.fn< + Parameters, + ReturnType + >() + baseBaseQuery + .mockResolvedValueOnce({ error: 'returned error' }) // HandledError + .mockRejectedValueOnce(new Error('thrown error')) // Not HandledError + .mockResolvedValueOnce({ error: 'returned error' }) // HandledError + .mockResolvedValue({ data: { success: true } }) + + const baseQuery = retry(baseBaseQuery, { maxRetries: 5 }) + const api = createApi({ + baseQuery, + endpoints: (build) => ({ + q1: build.query({ + query: () => {}, + }), + }), + }) + + const storeRef = setupApiStore(api, undefined, { + withoutTestLifecycles: true, + }) + + storeRef.store.dispatch(api.endpoints.q1.initiate({})) + + await loopTimers(6) + + // Should eventually succeed after 4 attempts + expect(baseBaseQuery).toHaveBeenCalledTimes(4) + }) + + test('thrown errors with mutations should respect maxRetries', async () => { + const baseBaseQuery = vi.fn< + Parameters, + ReturnType + >() + // Simulate persistent network error + baseBaseQuery.mockRejectedValue(new Error('Connection refused')) + + const baseQuery = retry(baseBaseQuery, { maxRetries: 2 }) + const api = createApi({ + baseQuery, + endpoints: (build) => ({ + m1: build.mutation({ + query: () => ({ method: 'POST' }), + }), + }), + }) + + const storeRef = setupApiStore(api, undefined, { + withoutTestLifecycles: true, + }) + + storeRef.store.dispatch(api.endpoints.m1.initiate({})) + + await loopTimers(4) + + // Should try initial + 2 retries = 3 total, then stop + // Currently this will fail because it retries infinitely + expect(baseBaseQuery).toHaveBeenCalledTimes(3) + }) }) From 9cb706e4fa0649d20d690549eede5044b3bfdc27 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Tue, 21 Oct 2025 21:56:01 -0400 Subject: [PATCH 2/5] Add Vitest deps to graphql-query --- packages/rtk-query-graphql-request-base-query/package.json | 4 +++- yarn.lock | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/rtk-query-graphql-request-base-query/package.json b/packages/rtk-query-graphql-request-base-query/package.json index 6682ac464c..ee4424dde2 100644 --- a/packages/rtk-query-graphql-request-base-query/package.json +++ b/packages/rtk-query-graphql-request-base-query/package.json @@ -32,10 +32,12 @@ }, "devDependencies": { "@reduxjs/toolkit": "^1.6.0 || ^2.0.0", + "@types/node": "^20.11.0", "graphql": "^16.5.0", "microbundle": "^0.13.3", "rimraf": "^3.0.2", - "typescript": "^5.8.2" + "typescript": "^5.8.2", + "vitest": "^1.6.0" }, "publishConfig": { "access": "public" diff --git a/yarn.lock b/yarn.lock index 9200516b2f..de0a2b10f4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8155,11 +8155,13 @@ __metadata: resolution: "@rtk-query/graphql-request-base-query@workspace:packages/rtk-query-graphql-request-base-query" dependencies: "@reduxjs/toolkit": "npm:^1.6.0 || ^2.0.0" + "@types/node": "npm:^20.11.0" graphql: "npm:^16.5.0" graphql-request: "npm:^4.0.0 || ^5.0.0 || ^6.0.0" microbundle: "npm:^0.13.3" rimraf: "npm:^3.0.2" typescript: "npm:^5.8.2" + vitest: "npm:^1.6.0" peerDependencies: "@reduxjs/toolkit": ^1.7.1 || ^2.0.0 graphql: ^0.8.0 || ^0.9.0 || ^0.10.0 || ^0.11.0 || ^0.12.0 || ^0.13.0 || ^14.0.0 || ^15.0.0 || ^16.0.0 From 9d7a6ff405a94901319837c2fe07329b2588d392 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Tue, 21 Oct 2025 21:56:31 -0400 Subject: [PATCH 3/5] Add Vitest config and update TS config --- .../tsconfig.json | 83 ++++--------------- .../vitest.config.mts | 26 ++++++ 2 files changed, 40 insertions(+), 69 deletions(-) create mode 100644 packages/rtk-query-graphql-request-base-query/vitest.config.mts diff --git a/packages/rtk-query-graphql-request-base-query/tsconfig.json b/packages/rtk-query-graphql-request-base-query/tsconfig.json index 1e6ed61d6e..16f42f34e4 100644 --- a/packages/rtk-query-graphql-request-base-query/tsconfig.json +++ b/packages/rtk-query-graphql-request-base-query/tsconfig.json @@ -1,73 +1,18 @@ { "compilerOptions": { - /* Visit https://aka.ms/tsconfig.json to read more about this file */ - - /* Basic Options */ - // "incremental": true, /* Enable incremental compilation */ - "target": "es5" /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017', 'ES2018', 'ES2019', 'ES2020', 'ES2021', or 'ESNEXT'. */, - "module": "commonjs" /* Specify module code generation: 'none', 'commonjs', 'amd', 'system', 'umd', 'es2015', 'es2020', or 'ESNext'. */, - // "lib": [], /* Specify library files to be included in the compilation. */ - // "allowJs": true, /* Allow javascript files to be compiled. */ - // "checkJs": true, /* Report errors in .js files. */ - // "jsx": "preserve", /* Specify JSX code generation: 'preserve', 'react-native', 'react', 'react-jsx' or 'react-jsxdev'. */ - // "declaration": true, /* Generates corresponding '.d.ts' file. */ - // "declarationMap": true, /* Generates a sourcemap for each corresponding '.d.ts' file. */ - // "sourceMap": true, /* Generates corresponding '.map' file. */ - // "outFile": "./", /* Concatenate and emit output to single file. */ - // "outDir": "./", /* Redirect output structure to the directory. */ - // "rootDir": "./", /* Specify the root directory of input files. Use to control the output directory structure with --outDir. */ - // "composite": true, /* Enable project compilation */ - // "tsBuildInfoFile": "./", /* Specify file to store incremental compilation information */ - // "removeComments": true, /* Do not emit comments to output. */ - // "noEmit": true, /* Do not emit outputs. */ - // "importHelpers": true, /* Import emit helpers from 'tslib'. */ - // "downlevelIteration": true, /* Provide full support for iterables in 'for-of', spread, and destructuring when targeting 'ES5' or 'ES3'. */ - // "isolatedModules": true, /* Transpile each file as a separate module (similar to 'ts.transpileModule'). */ - - /* Strict Type-Checking Options */ - "strict": true /* Enable all strict type-checking options. */, - // "noImplicitAny": true, /* Raise error on expressions and declarations with an implied 'any' type. */ - // "strictNullChecks": true, /* Enable strict null checks. */ - // "strictFunctionTypes": true, /* Enable strict checking of function types. */ - // "strictBindCallApply": true, /* Enable strict 'bind', 'call', and 'apply' methods on functions. */ - // "strictPropertyInitialization": true, /* Enable strict checking of property initialization in classes. */ - // "noImplicitThis": true, /* Raise error on 'this' expressions with an implied 'any' type. */ - // "alwaysStrict": true, /* Parse in strict mode and emit "use strict" for each source file. */ - - /* Additional Checks */ - // "noUnusedLocals": true, /* Report errors on unused locals. */ - // "noUnusedParameters": true, /* Report errors on unused parameters. */ - // "noImplicitReturns": true, /* Report error when not all code paths in function return a value. */ - // "noFallthroughCasesInSwitch": true, /* Report errors for fallthrough cases in switch statement. */ - // "noUncheckedIndexedAccess": true, /* Include 'undefined' in index signature results */ - // "noImplicitOverride": true, /* Ensure overriding members in derived classes are marked with an 'override' modifier. */ - // "noPropertyAccessFromIndexSignature": true, /* Require undeclared properties from index signatures to use element accesses. */ - - /* Module Resolution Options */ - // "moduleResolution": "node", /* Specify module resolution strategy: 'node' (Node.js) or 'classic' (TypeScript pre-1.6). */ - // "baseUrl": "./", /* Base directory to resolve non-absolute module names. */ - // "paths": {}, /* A series of entries which re-map imports to lookup locations relative to the 'baseUrl'. */ - // "rootDirs": [], /* List of root folders whose combined content represents the structure of the project at runtime. */ - // "typeRoots": [], /* List of folders to include type definitions from. */ - // "types": [], /* Type declaration files to be included in compilation. */ - // "allowSyntheticDefaultImports": true, /* Allow default imports from modules with no default export. This does not affect code emit, just typechecking. */ - "esModuleInterop": true /* Enables emit interoperability between CommonJS and ES Modules via creation of namespace objects for all imports. Implies 'allowSyntheticDefaultImports'. */, - // "preserveSymlinks": true, /* Do not resolve the real path of symlinks. */ - // "allowUmdGlobalAccess": true, /* Allow accessing UMD globals from modules. */ - - /* Source Map Options */ - // "sourceRoot": "", /* Specify the location where debugger should locate TypeScript files instead of source locations. */ - // "mapRoot": "", /* Specify the location where debugger should locate map files instead of generated locations. */ - // "inlineSourceMap": true, /* Emit a single file with source maps instead of having a separate file. */ - // "inlineSources": true, /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */ - - /* Experimental Options */ - // "experimentalDecorators": true, /* Enables experimental support for ES7 decorators. */ - // "emitDecoratorMetadata": true, /* Enables experimental support for emitting type metadata for decorators. */ - - /* Advanced Options */ - "skipLibCheck": true /* Skip type checking of declaration files. */, - "forceConsistentCasingInFileNames": true /* Disallow inconsistently-cased references to the same file. */ + "target": "ES2020", + "module": "ESNext", + "lib": ["ES2020"], + "moduleResolution": "bundler", + "strict": true, + "esModuleInterop": true, + "skipLibCheck": true, + "forceConsistentCasingInFileNames": true, + "declaration": true, + "declarationMap": true, + "sourceMap": true, + "types": ["vitest/globals"] }, - "include": ["**/*.ts"] + "include": ["src/**/*"], + "exclude": ["node_modules", "dist"] } diff --git a/packages/rtk-query-graphql-request-base-query/vitest.config.mts b/packages/rtk-query-graphql-request-base-query/vitest.config.mts new file mode 100644 index 0000000000..90f9a6d019 --- /dev/null +++ b/packages/rtk-query-graphql-request-base-query/vitest.config.mts @@ -0,0 +1,26 @@ +import path from 'node:path' +import { fileURLToPath } from 'node:url' +import { defineConfig } from 'vitest/config' + +// No __dirname under Node ESM +const __filename = fileURLToPath(import.meta.url) +const __dirname = path.dirname(__filename) + +export default defineConfig({ + test: { + globals: true, + environment: 'node', // Node environment since this is a baseQuery, not React + include: ['./src/**/*.test.ts'], + coverage: { + provider: 'v8', + reporter: ['text', 'json', 'html'], + include: ['src/**/*.ts'], + exclude: ['src/**/*.test.ts', 'src/**/*.d.ts'], + }, + }, + resolve: { + alias: { + '@reduxjs/toolkit': path.resolve(__dirname, '../toolkit/src'), + }, + }, +}) From 36c5c504d1ba9f13465b9bb84633ad5bd44a9aaf Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Tue, 21 Oct 2025 21:56:46 -0400 Subject: [PATCH 4/5] Add graphql-request tests --- .../package.json | 3 +- .../src/index.test.ts | 420 ++++++++++++++++++ 2 files changed, 422 insertions(+), 1 deletion(-) create mode 100644 packages/rtk-query-graphql-request-base-query/src/index.test.ts diff --git a/packages/rtk-query-graphql-request-base-query/package.json b/packages/rtk-query-graphql-request-base-query/package.json index ee4424dde2..d190ba0620 100644 --- a/packages/rtk-query-graphql-request-base-query/package.json +++ b/packages/rtk-query-graphql-request-base-query/package.json @@ -21,7 +21,8 @@ "build": "microbundle", "prepack": "rimraf dist/*; yarn build", "dev": "microbundle watch", - "test": "true" + "test": "vitest run", + "test:watch": "vitest" }, "dependencies": { "graphql-request": "^4.0.0 || ^5.0.0 || ^6.0.0" diff --git a/packages/rtk-query-graphql-request-base-query/src/index.test.ts b/packages/rtk-query-graphql-request-base-query/src/index.test.ts new file mode 100644 index 0000000000..3e763bdb65 --- /dev/null +++ b/packages/rtk-query-graphql-request-base-query/src/index.test.ts @@ -0,0 +1,420 @@ +import { describe, it, expect, vi } from 'vitest' +import type { GraphQLClient } from 'graphql-request' +import { ClientError } from 'graphql-request' +import { graphqlRequestBaseQuery } from './index' +import type { BaseQueryApi } from '@reduxjs/toolkit/query' + +describe('graphqlRequestBaseQuery', () => { + const mockGetState = vi.fn() + const mockDispatch = vi.fn() + const mockAbort = vi.fn() + const mockExtra = {} + const mockSignal = new AbortController().signal + + const createMockApi = (): BaseQueryApi => ({ + signal: mockSignal, + abort: mockAbort, + dispatch: mockDispatch, + getState: mockGetState, + extra: mockExtra, + endpoint: 'testEndpoint', + type: 'query' as const, + forced: false, + }) + + describe('successful requests', () => { + it('returns data on successful request', async () => { + const mockData = { user: { id: '1', name: 'Test User' } } + const mockClient = { + request: vi.fn().mockResolvedValue(mockData), + } as unknown as GraphQLClient + + const baseQuery = graphqlRequestBaseQuery({ client: mockClient }) + const result = await baseQuery( + { + document: 'query { user { id name } }', + variables: {}, + }, + createMockApi(), + {}, + ) + + expect(result).toEqual({ + data: mockData, + meta: {}, + }) + expect(mockClient.request).toHaveBeenCalledTimes(1) + }) + + it('passes variables to the GraphQL client', async () => { + const mockData = { user: { id: '1' } } + const mockClient = { + request: vi.fn().mockResolvedValue(mockData), + } as unknown as GraphQLClient + + const baseQuery = graphqlRequestBaseQuery({ client: mockClient }) + const variables = { userId: '1' } + + await baseQuery( + { + document: 'query($userId: ID!) { user(id: $userId) { id } }', + variables, + }, + createMockApi(), + {}, + ) + + expect(mockClient.request).toHaveBeenCalledWith( + expect.objectContaining({ + variables, + }), + ) + }) + + it('passes document to the GraphQL client', async () => { + const mockData = { test: 'data' } + const mockClient = { + request: vi.fn().mockResolvedValue(mockData), + } as unknown as GraphQLClient + + const baseQuery = graphqlRequestBaseQuery({ client: mockClient }) + const document = 'query { test }' + + await baseQuery( + { + document, + }, + createMockApi(), + {}, + ) + + expect(mockClient.request).toHaveBeenCalledWith( + expect.objectContaining({ + document, + }), + ) + }) + }) + + describe('error handling', () => { + it('returns error for ClientError (GraphQL API errors)', async () => { + const mockError = new ClientError( + { + errors: [{ message: 'User not found' }], + } as any, + { query: 'test query' } as any, + ) + + const mockClient = { + request: vi.fn().mockRejectedValue(mockError), + } as unknown as GraphQLClient + + const baseQuery = graphqlRequestBaseQuery({ client: mockClient }) + const result = await baseQuery( + { + document: 'query { user { id } }', + }, + createMockApi(), + {}, + ) + + expect(result.error).toBeDefined() + expect(result.error).toHaveProperty('name') + expect(result.error).toHaveProperty('message') + expect(result.meta).toBeDefined() + }) + + it('includes request and response in meta for ClientError', async () => { + const mockRequest = { query: 'test query' } + const mockResponse = { errors: [{ message: 'Error' }] } + const mockError = new ClientError(mockResponse as any, mockRequest as any) + + const mockClient = { + request: vi.fn().mockRejectedValue(mockError), + } as unknown as GraphQLClient + + const baseQuery = graphqlRequestBaseQuery({ client: mockClient }) + const result = await baseQuery( + { + document: 'query { test }', + }, + createMockApi(), + {}, + ) + + expect(result.meta).toHaveProperty('request') + expect(result.meta).toHaveProperty('response') + }) + + it('throws network errors (non-ClientError) - documents current bug', async () => { + const networkError = new Error('Network timeout') + const mockClient = { + request: vi.fn().mockRejectedValue(networkError), + } as unknown as GraphQLClient + + const baseQuery = graphqlRequestBaseQuery({ client: mockClient }) + + // Currently throws - this test documents the bug that causes infinite retries + await expect( + baseQuery( + { + document: 'query { user { id } }', + }, + createMockApi(), + {}, + ), + ).rejects.toThrow('Network timeout') + }) + + it('throws fetch errors - documents current bug', async () => { + const fetchError = new Error('Failed to fetch') + const mockClient = { + request: vi.fn().mockRejectedValue(fetchError), + } as unknown as GraphQLClient + + const baseQuery = graphqlRequestBaseQuery({ client: mockClient }) + + await expect( + baseQuery( + { + document: 'query { test }', + }, + createMockApi(), + {}, + ), + ).rejects.toThrow('Failed to fetch') + }) + + it('uses custom error handler when provided', async () => { + const mockError = new ClientError( + { + errors: [{ message: 'Custom error' }], + } as any, + { query: 'test query' } as any, + ) + + const mockClient = { + request: vi.fn().mockRejectedValue(mockError), + } as unknown as GraphQLClient + + const customErrors = vi.fn((error: ClientError) => ({ + customField: 'custom value', + originalMessage: error.message, + })) + + const baseQuery = graphqlRequestBaseQuery({ + client: mockClient, + customErrors, + }) + + const result = await baseQuery( + { + document: 'query { user { id } }', + }, + createMockApi(), + {}, + ) + + expect(customErrors).toHaveBeenCalledWith(mockError) + expect(result.error).toEqual({ + customField: 'custom value', + originalMessage: expect.any(String), + }) + }) + + it('uses default error format when customErrors not provided', async () => { + const mockError = new ClientError( + { + errors: [{ message: 'Test error' }], + } as any, + { query: 'test query' } as any, + ) + + const mockClient = { + request: vi.fn().mockRejectedValue(mockError), + } as unknown as GraphQLClient + + const baseQuery = graphqlRequestBaseQuery({ client: mockClient }) + const result = await baseQuery( + { + document: 'query { test }', + }, + createMockApi(), + {}, + ) + + expect(result.error).toHaveProperty('name') + expect(result.error).toHaveProperty('message') + expect(result.error).toHaveProperty('stack') + }) + }) + + describe('headers', () => { + it('applies prepareHeaders function', async () => { + const mockData = { test: 'data' } + const mockClient = { + request: vi.fn().mockResolvedValue(mockData), + } as unknown as GraphQLClient + + const prepareHeaders = vi.fn((headers: Headers) => { + headers.set('Authorization', 'Bearer token123') + return headers + }) + + const baseQuery = graphqlRequestBaseQuery({ + client: mockClient, + prepareHeaders, + }) + + await baseQuery( + { + document: 'query { test }', + }, + createMockApi(), + {}, + ) + + expect(prepareHeaders).toHaveBeenCalled() + expect(mockClient.request).toHaveBeenCalledWith( + expect.objectContaining({ + requestHeaders: expect.any(Headers), + }), + ) + }) + + it('passes correct arguments to prepareHeaders', async () => { + const mockData = { test: 'data' } + const mockClient = { + request: vi.fn().mockResolvedValue(mockData), + } as unknown as GraphQLClient + + const prepareHeaders = vi.fn((headers: Headers) => headers) + + const baseQuery = graphqlRequestBaseQuery({ + client: mockClient, + prepareHeaders, + }) + + const mockApi = createMockApi() + await baseQuery( + { + document: 'query { test }', + }, + mockApi, + {}, + ) + + expect(prepareHeaders).toHaveBeenCalledWith( + expect.any(Headers), + expect.objectContaining({ + getState: mockApi.getState, + endpoint: mockApi.endpoint, + forced: mockApi.forced, + type: mockApi.type, + extra: mockApi.extra, + }), + ) + }) + + it('merges requestHeaders with prepareHeaders', async () => { + const mockData = { test: 'data' } + const mockClient = { + request: vi.fn().mockResolvedValue(mockData), + } as unknown as GraphQLClient + + const prepareHeaders = vi.fn((headers: Headers) => { + headers.set('X-Custom-Header', 'custom-value') + return headers + }) + + const baseQuery = graphqlRequestBaseQuery({ + client: mockClient, + requestHeaders: { + 'Content-Type': 'application/json', + }, + prepareHeaders, + }) + + await baseQuery( + { + document: 'query { test }', + }, + createMockApi(), + {}, + ) + + const callArgs = (mockClient.request as any).mock.calls[0][0] + const headers = callArgs.requestHeaders as Headers + + expect(headers.get('Content-Type')).toBe('application/json') + expect(headers.get('X-Custom-Header')).toBe('custom-value') + }) + }) + + describe('client initialization', () => { + it('creates GraphQLClient from URL when client not provided', async () => { + const mockData = { test: 'data' } + + // We can't easily test the actual client creation without mocking the constructor, + // but we can verify the baseQuery works with a URL + const baseQuery = graphqlRequestBaseQuery({ + url: 'https://api.example.com/graphql', + }) + + // This will fail in the test environment since there's no actual server, + // but it verifies the configuration is accepted + expect(baseQuery).toBeDefined() + expect(typeof baseQuery).toBe('function') + }) + + it('uses provided client when available', async () => { + const mockData = { test: 'data' } + const mockClient = { + request: vi.fn().mockResolvedValue(mockData), + } as unknown as GraphQLClient + + const baseQuery = graphqlRequestBaseQuery({ client: mockClient }) + + await baseQuery( + { + document: 'query { test }', + }, + createMockApi(), + {}, + ) + + expect(mockClient.request).toHaveBeenCalled() + }) + }) + + describe('signal handling', () => { + it('passes abort signal to GraphQL client', async () => { + const mockData = { test: 'data' } + const mockClient = { + request: vi.fn().mockResolvedValue(mockData), + } as unknown as GraphQLClient + + const baseQuery = graphqlRequestBaseQuery({ client: mockClient }) + const controller = new AbortController() + + const mockApi = { + ...createMockApi(), + signal: controller.signal, + } + + await baseQuery( + { + document: 'query { test }', + }, + mockApi, + {}, + ) + + expect(mockClient.request).toHaveBeenCalledWith( + expect.objectContaining({ + signal: controller.signal, + }), + ) + }) + }) +}) From d70646b3fc72a151bda249bdf5dd71ab8125966a Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Tue, 21 Oct 2025 22:05:00 -0400 Subject: [PATCH 5/5] Actually return errors from `graphql-request` --- .../src/index.test.ts | 86 +++++++++++++------ .../src/index.ts | 15 +++- 2 files changed, 71 insertions(+), 30 deletions(-) diff --git a/packages/rtk-query-graphql-request-base-query/src/index.test.ts b/packages/rtk-query-graphql-request-base-query/src/index.test.ts index 3e763bdb65..99ba4be59f 100644 --- a/packages/rtk-query-graphql-request-base-query/src/index.test.ts +++ b/packages/rtk-query-graphql-request-base-query/src/index.test.ts @@ -118,10 +118,17 @@ describe('graphqlRequestBaseQuery', () => { {}, ) - expect(result.error).toBeDefined() - expect(result.error).toHaveProperty('name') - expect(result.error).toHaveProperty('message') - expect(result.meta).toBeDefined() + expect(result).toEqual({ + error: { + name: expect.any(String), + message: expect.any(String), + stack: expect.any(String), + }, + meta: { + request: expect.any(Object), + response: expect.any(Object), + }, + }) }) it('includes request and response in meta for ClientError', async () => { @@ -146,7 +153,7 @@ describe('graphqlRequestBaseQuery', () => { expect(result.meta).toHaveProperty('response') }) - it('throws network errors (non-ClientError) - documents current bug', async () => { + it('returns network errors (non-ClientError) instead of throwing', async () => { const networkError = new Error('Network timeout') const mockClient = { request: vi.fn().mockRejectedValue(networkError), @@ -154,19 +161,26 @@ describe('graphqlRequestBaseQuery', () => { const baseQuery = graphqlRequestBaseQuery({ client: mockClient }) - // Currently throws - this test documents the bug that causes infinite retries - await expect( - baseQuery( - { - document: 'query { user { id } }', - }, - createMockApi(), - {}, - ), - ).rejects.toThrow('Network timeout') + const result = await baseQuery( + { + document: 'query { user { id } }', + }, + createMockApi(), + {}, + ) + + // Should return error instead of throwing + expect(result).toEqual({ + error: { + name: 'Error', + message: 'Network timeout', + stack: expect.any(String), + }, + meta: {}, + }) }) - it('throws fetch errors - documents current bug', async () => { + it('returns fetch errors instead of throwing', async () => { const fetchError = new Error('Failed to fetch') const mockClient = { request: vi.fn().mockRejectedValue(fetchError), @@ -174,15 +188,23 @@ describe('graphqlRequestBaseQuery', () => { const baseQuery = graphqlRequestBaseQuery({ client: mockClient }) - await expect( - baseQuery( - { - document: 'query { test }', - }, - createMockApi(), - {}, - ), - ).rejects.toThrow('Failed to fetch') + const result = await baseQuery( + { + document: 'query { test }', + }, + createMockApi(), + {}, + ) + + // Should return error instead of throwing + expect(result).toEqual({ + error: { + name: 'Error', + message: 'Failed to fetch', + stack: expect.any(String), + }, + meta: {}, + }) }) it('uses custom error handler when provided', async () => { @@ -243,9 +265,17 @@ describe('graphqlRequestBaseQuery', () => { {}, ) - expect(result.error).toHaveProperty('name') - expect(result.error).toHaveProperty('message') - expect(result.error).toHaveProperty('stack') + expect(result).toEqual({ + error: { + name: expect.any(String), + message: expect.any(String), + stack: expect.any(String), + }, + meta: { + request: expect.any(Object), + response: expect.any(Object), + }, + }) }) }) diff --git a/packages/rtk-query-graphql-request-base-query/src/index.ts b/packages/rtk-query-graphql-request-base-query/src/index.ts index b796c5d6d8..8e19150881 100644 --- a/packages/rtk-query-graphql-request-base-query/src/index.ts +++ b/packages/rtk-query-graphql-request-base-query/src/index.ts @@ -1,7 +1,8 @@ import { isPlainObject } from '@reduxjs/toolkit' import type { BaseQueryFn } from '@reduxjs/toolkit/query' import type { DocumentNode } from 'graphql' -import { GraphQLClient, ClientError, RequestOptions } from 'graphql-request' +import type { RequestOptions } from 'graphql-request' +import { GraphQLClient, ClientError } from 'graphql-request' import type { ErrorResponse, GraphqlRequestBaseQueryArgs, @@ -59,7 +60,17 @@ export const graphqlRequestBaseQuery = ( return { error: customizedErrors, meta: { request, response } } } - throw error + // Base queries should never throw, but return {error}. + // This also ensures that retry logic works correctly. + const err = error as Error + return { + error: { + name: err?.name || 'NetworkError', + message: err?.message || 'An unknown network error occurred', + stack: err?.stack, + } as E, + meta: {}, + } } } }