Skip to content

Commit d08fc94

Browse files
authored
fix: sign out correctly on error (#1452)
This actually doesn't fix the error but should make it more flexible. Also adds some testing. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit * **Refactor** * Enhanced error detection for invalid API key messages to improve accuracy. * **Tests** * Added comprehensive tests verifying error handling and client lifecycle behavior without network dependencies. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 7c6f02a commit d08fc94

File tree

2 files changed

+161
-1
lines changed

2 files changed

+161
-1
lines changed

packages/unraid-api-plugin-connect/src/service/graphql.client.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ export class MothershipGraphqlClientService implements OnModuleInit, OnModuleDes
184184
* Check if an error is an invalid API key error
185185
*/
186186
private isInvalidApiKeyError(error: unknown): boolean {
187-
return error instanceof Error && error.message.includes('API Key Invalid');
187+
return typeof error === 'object' && error !== null && 'message' in error && typeof error.message === 'string' && error.message.includes('API Key Invalid');
188188
}
189189

190190
/**
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
3+
import { MothershipGraphqlClientService } from '../service/graphql.client.js';
4+
import { MinigraphStatus } from '../model/connect-config.model.js';
5+
6+
// Mock only the WebSocket client creation, not the Apollo Client error handling
7+
vi.mock('graphql-ws', () => ({
8+
createClient: vi.fn(),
9+
}));
10+
11+
// Mock WebSocket to avoid actual network connections
12+
vi.mock('ws', () => ({
13+
WebSocket: vi.fn().mockImplementation(() => ({})),
14+
}));
15+
16+
describe('MothershipGraphqlClientService', () => {
17+
let service: MothershipGraphqlClientService;
18+
let mockConfigService: any;
19+
let mockConnectionService: any;
20+
let mockEventEmitter: any;
21+
let mockWsClient: any;
22+
23+
beforeEach(async () => {
24+
vi.clearAllMocks();
25+
26+
mockConfigService = {
27+
getOrThrow: vi.fn((key: string) => {
28+
switch (key) {
29+
case 'API_VERSION':
30+
return '4.8.0+test';
31+
case 'MOTHERSHIP_GRAPHQL_LINK':
32+
return 'https://mothership.unraid.net/ws';
33+
default:
34+
throw new Error(`Unknown config key: ${key}`);
35+
}
36+
}),
37+
set: vi.fn(),
38+
};
39+
40+
mockConnectionService = {
41+
getIdentityState: vi.fn().mockReturnValue({ isLoaded: true }),
42+
getWebsocketConnectionParams: vi.fn().mockReturnValue({}),
43+
getMothershipWebsocketHeaders: vi.fn().mockReturnValue({}),
44+
getConnectionState: vi.fn().mockReturnValue({ status: MinigraphStatus.CONNECTED }),
45+
setConnectionStatus: vi.fn(),
46+
receivePing: vi.fn(),
47+
};
48+
49+
mockEventEmitter = {
50+
emit: vi.fn(),
51+
};
52+
53+
mockWsClient = {
54+
on: vi.fn().mockReturnValue(() => {}),
55+
terminate: vi.fn(),
56+
dispose: vi.fn().mockResolvedValue(undefined),
57+
};
58+
59+
// Mock the createClient function
60+
const { createClient } = await import('graphql-ws');
61+
vi.mocked(createClient).mockReturnValue(mockWsClient as any);
62+
63+
service = new MothershipGraphqlClientService(
64+
mockConfigService as any,
65+
mockConnectionService as any,
66+
mockEventEmitter as any
67+
);
68+
});
69+
70+
describe('isInvalidApiKeyError', () => {
71+
it.each([
72+
{
73+
description: 'standard API key error',
74+
error: { message: 'API Key Invalid with error No user found' },
75+
expected: true,
76+
},
77+
{
78+
description: 'simple API key error',
79+
error: { message: 'API Key Invalid' },
80+
expected: true,
81+
},
82+
{
83+
description: 'API key error within other text',
84+
error: { message: 'Something else API Key Invalid something' },
85+
expected: true,
86+
},
87+
{
88+
description: 'malformed GraphQL error with API key message',
89+
error: {
90+
message: '"error" message expects the \'payload\' property to be an array of GraphQL errors, but got "API Key Invalid with error No user found"',
91+
},
92+
expected: true,
93+
},
94+
{
95+
description: 'non-API key error',
96+
error: { message: 'Network connection failed' },
97+
expected: false,
98+
},
99+
{
100+
description: 'null error',
101+
error: null,
102+
expected: false,
103+
},
104+
{
105+
description: 'empty error object',
106+
error: {},
107+
expected: false,
108+
},
109+
])('should identify $description correctly', ({ error, expected }) => {
110+
const isInvalidApiKeyError = (service as any).isInvalidApiKeyError.bind(service);
111+
expect(isInvalidApiKeyError(error)).toBe(expected);
112+
});
113+
});
114+
115+
describe('client lifecycle', () => {
116+
it('should return null client when identity state is not valid', () => {
117+
mockConnectionService.getIdentityState.mockReturnValue({ isLoaded: false });
118+
119+
const client = service.getClient();
120+
121+
expect(client).toBeNull();
122+
});
123+
124+
it('should return client when identity state is valid', () => {
125+
mockConnectionService.getIdentityState.mockReturnValue({ isLoaded: true });
126+
127+
// Since we're not mocking Apollo Client, this will create a real client
128+
// We just want to verify the state check works
129+
const client = service.getClient();
130+
131+
// The client should either be null (if not created yet) or an Apollo client instance
132+
// The key is that it doesn't throw an error when state is valid
133+
expect(() => service.getClient()).not.toThrow();
134+
});
135+
});
136+
137+
describe('sendQueryResponse', () => {
138+
it('should handle null client gracefully', async () => {
139+
// Make identity state invalid so getClient returns null
140+
mockConnectionService.getIdentityState.mockReturnValue({ isLoaded: false });
141+
142+
const result = await service.sendQueryResponse('test-sha256', {
143+
data: { test: 'data' },
144+
});
145+
146+
// Should not throw and should return undefined when client is null
147+
expect(result).toBeUndefined();
148+
});
149+
});
150+
151+
describe('configuration', () => {
152+
it('should get API version from config', () => {
153+
expect(service.apiVersion).toBe('4.8.0+test');
154+
});
155+
156+
it('should get mothership GraphQL link from config', () => {
157+
expect(service.mothershipGraphqlLink).toBe('https://mothership.unraid.net/ws');
158+
});
159+
});
160+
});

0 commit comments

Comments
 (0)