Skip to content

Commit 849eebd

Browse files
authored
Merge pull request #814 from objectstack-ai/copilot/fix-async-issues-in-dispatcher
2 parents 6427aba + 231d399 commit 849eebd

3 files changed

Lines changed: 283 additions & 17 deletions

File tree

ROADMAP.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# ObjectStack Protocol — Road Map
22

3-
> **Last Updated:** 2026-02-24
3+
> **Last Updated:** 2026-02-25
44
> **Current Version:** v3.0.8
55
> **Status:** Protocol Specification Complete · Runtime Implementation In Progress
66
@@ -123,6 +123,7 @@ This strategy ensures rapid iteration while maintaining a clear path to producti
123123
| `z.unknown()` in extensibility fields | ✅ Justified | `properties`, `children`, `context`, `options`, `body` — inherently dynamic extensibility points |
124124
| DashboardWidget discriminated union by type | 🔴 | Planned — chart/metric/pivot subtypes with type-specific required fields |
125125
| CI lint rule rejecting new `z.any()` | 🔴 | Planned — eslint or custom lint rule to block `z.any()` additions |
126+
| Dispatcher async `getService` bug fix || All `getService`/`getObjectQLService` calls in `http-dispatcher.ts` now properly `await` async service factories. Covers `handleAnalytics`, `handleAuth`, `handleStorage`, `handleAutomation`, `handleMetadata`, `handleUi`, `handlePackages`. 20 new tests for async/sync/error scenarios. |
126127

127128
---
128129

packages/runtime/src/http-dispatcher.test.ts

Lines changed: 265 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,4 +204,269 @@ describe('HttpDispatcher', () => {
204204
expect(mockAutomationService.trigger).toHaveBeenCalledWith('flow_a', { data: 1 }, { request: {} });
205205
});
206206
});
207+
208+
// ═══════════════════════════════════════════════════════════════
209+
// Async Service Resolution Tests
210+
// Covers: getService awaits Promise-based (async factory) services
211+
// ═══════════════════════════════════════════════════════════════
212+
213+
describe('Async service resolution (Promise-based injection)', () => {
214+
215+
describe('handleAnalytics with async service', () => {
216+
it('should resolve analytics service from Promise (async factory)', async () => {
217+
const mockAnalytics = {
218+
query: vi.fn().mockResolvedValue({ rows: [{ id: 1 }], total: 1 }),
219+
getMetadata: vi.fn().mockResolvedValue({ tables: ['t1'] }),
220+
generateSql: vi.fn().mockResolvedValue({ sql: 'SELECT 1' }),
221+
};
222+
// Inject as Promise (simulates async factory registration)
223+
(kernel as any).getService = vi.fn().mockImplementation((name: string) => {
224+
if (name === 'analytics') return Promise.resolve(mockAnalytics);
225+
return null;
226+
});
227+
228+
const result = await dispatcher.handleAnalytics('query', 'POST', { sql: 'SELECT 1' }, { request: {} });
229+
expect(result.handled).toBe(true);
230+
expect(result.response?.status).toBe(200);
231+
expect(mockAnalytics.query).toHaveBeenCalled();
232+
});
233+
234+
it('should handle POST /analytics/sql with async service', async () => {
235+
const mockAnalytics = {
236+
generateSql: vi.fn().mockResolvedValue({ sql: 'SELECT * FROM t' }),
237+
};
238+
(kernel as any).getService = vi.fn().mockResolvedValue(mockAnalytics);
239+
240+
const result = await dispatcher.handleAnalytics('sql', 'POST', { object: 'test' }, { request: {} });
241+
expect(result.handled).toBe(true);
242+
expect(result.response?.status).toBe(200);
243+
expect(mockAnalytics.generateSql).toHaveBeenCalled();
244+
});
245+
246+
it('should handle GET /analytics/meta with async service', async () => {
247+
const mockAnalytics = {
248+
getMetadata: vi.fn().mockResolvedValue({ tables: ['users', 'orders'] }),
249+
};
250+
(kernel as any).getService = vi.fn().mockResolvedValue(mockAnalytics);
251+
252+
const result = await dispatcher.handleAnalytics('meta', 'GET', {}, { request: {} });
253+
expect(result.handled).toBe(true);
254+
expect(result.response?.status).toBe(200);
255+
expect(result.response?.body?.data?.tables).toEqual(['users', 'orders']);
256+
});
257+
258+
it('should return unhandled when analytics service is not registered', async () => {
259+
(kernel as any).getService = vi.fn().mockResolvedValue(null);
260+
(kernel as any).services = new Map();
261+
262+
const result = await dispatcher.handleAnalytics('query', 'POST', {}, { request: {} });
263+
expect(result.handled).toBe(false);
264+
});
265+
266+
it('should return unhandled for unknown analytics sub-path', async () => {
267+
const mockAnalytics = { query: vi.fn() };
268+
(kernel as any).getService = vi.fn().mockResolvedValue(mockAnalytics);
269+
270+
const result = await dispatcher.handleAnalytics('unknown', 'POST', {}, { request: {} });
271+
expect(result.handled).toBe(false);
272+
});
273+
});
274+
275+
describe('handleAuth with async service', () => {
276+
it('should resolve auth service from Promise', async () => {
277+
const mockAuth = {
278+
handler: vi.fn().mockResolvedValue({ user: { id: '1' } }),
279+
};
280+
(kernel as any).getService = vi.fn().mockImplementation((name: string) => {
281+
if (name === 'auth') return Promise.resolve(mockAuth);
282+
return null;
283+
});
284+
285+
const result = await dispatcher.handleAuth('', 'POST', {}, { request: {}, response: {} });
286+
expect(result.handled).toBe(true);
287+
expect(mockAuth.handler).toHaveBeenCalled();
288+
});
289+
290+
it('should fallback to legacy login when async auth service has no handler', async () => {
291+
(kernel as any).getService = vi.fn().mockResolvedValue({});
292+
mockBroker.call.mockResolvedValue({ token: 'abc' });
293+
294+
const result = await dispatcher.handleAuth('/login', 'POST', { user: 'a' }, { request: {} });
295+
expect(result.handled).toBe(true);
296+
expect(mockBroker.call).toHaveBeenCalledWith('auth.login', { user: 'a' }, { request: {} });
297+
});
298+
299+
it('should return unhandled when auth service not registered and no legacy match', async () => {
300+
(kernel as any).getService = vi.fn().mockResolvedValue(null);
301+
(kernel as any).services = new Map();
302+
303+
const result = await dispatcher.handleAuth('/profile', 'GET', {}, { request: {} });
304+
expect(result.handled).toBe(false);
305+
});
306+
});
307+
308+
describe('handleStorage with async service', () => {
309+
it('should resolve storage service from Promise', async () => {
310+
const mockStorage = {
311+
upload: vi.fn().mockResolvedValue({ id: 'file_1', url: '/files/1' }),
312+
};
313+
(kernel as any).getService = vi.fn().mockImplementation((name: string) => {
314+
if (name === 'file-storage') return Promise.resolve(mockStorage);
315+
return null;
316+
});
317+
318+
const result = await dispatcher.handleStorage('/upload', 'POST', { name: 'test.txt' }, { request: {} });
319+
expect(result.handled).toBe(true);
320+
expect(result.response?.status).toBe(200);
321+
expect(mockStorage.upload).toHaveBeenCalled();
322+
});
323+
324+
it('should return 501 when storage service is not registered (async null)', async () => {
325+
(kernel as any).getService = vi.fn().mockResolvedValue(null);
326+
(kernel as any).services = new Map();
327+
328+
const result = await dispatcher.handleStorage('/upload', 'POST', {}, { request: {} });
329+
expect(result.handled).toBe(true);
330+
expect(result.response?.status).toBe(501);
331+
expect(result.response?.body?.error?.message).toBe('File storage not configured');
332+
});
333+
334+
it('should handle GET /storage/file/:id with async service', async () => {
335+
const mockStorage = {
336+
download: vi.fn().mockResolvedValue({ data: 'content', mimeType: 'text/plain' }),
337+
};
338+
(kernel as any).getService = vi.fn().mockImplementation((name: string) => {
339+
if (name === 'file-storage') return Promise.resolve(mockStorage);
340+
return null;
341+
});
342+
343+
const result = await dispatcher.handleStorage('/file/abc123', 'GET', null, { request: {} });
344+
expect(result.handled).toBe(true);
345+
expect(mockStorage.download).toHaveBeenCalledWith('abc123', { request: {} });
346+
});
347+
348+
it('should return 400 when upload has no file', async () => {
349+
const mockStorage = { upload: vi.fn() };
350+
(kernel as any).getService = vi.fn().mockResolvedValue(mockStorage);
351+
352+
const result = await dispatcher.handleStorage('/upload', 'POST', null, { request: {} });
353+
expect(result.handled).toBe(true);
354+
expect(result.response?.status).toBe(400);
355+
expect(result.response?.body?.error?.message).toBe('No file provided');
356+
});
357+
});
358+
359+
describe('handleAutomation with async service', () => {
360+
it('should resolve automation service from Promise (async factory)', async () => {
361+
const mockAuto = {
362+
listFlows: vi.fn().mockResolvedValue(['f1']),
363+
};
364+
(kernel as any).getService = vi.fn().mockImplementation((name: string) => {
365+
if (name === 'automation') return Promise.resolve(mockAuto);
366+
return null;
367+
});
368+
369+
const result = await dispatcher.handleAutomation('', 'GET', {}, { request: {} });
370+
expect(result.handled).toBe(true);
371+
expect(result.response?.body?.data?.flows).toEqual(['f1']);
372+
});
373+
374+
it('should return unhandled when automation service not registered', async () => {
375+
(kernel as any).getService = vi.fn().mockResolvedValue(null);
376+
(kernel as any).services = new Map();
377+
378+
const result = await dispatcher.handleAutomation('', 'GET', {}, { request: {} });
379+
expect(result.handled).toBe(false);
380+
});
381+
});
382+
383+
describe('handleMetadata with async protocol service', () => {
384+
it('should resolve protocol service from async getService', async () => {
385+
const asyncProtocol = {
386+
saveMetaItem: vi.fn().mockResolvedValue({ success: true }),
387+
};
388+
(kernel as any).context.getService = vi.fn().mockImplementation((name: string) => {
389+
if (name === 'protocol') return Promise.resolve(asyncProtocol);
390+
return null;
391+
});
392+
393+
const result = await dispatcher.handleMetadata('/objects/my_obj', { request: {} }, 'PUT', { label: 'Test' });
394+
expect(result.handled).toBe(true);
395+
expect(result.response?.status).toBe(200);
396+
expect(asyncProtocol.saveMetaItem).toHaveBeenCalled();
397+
});
398+
399+
it('should fallback to broker when async protocol returns null', async () => {
400+
(kernel as any).context.getService = vi.fn().mockResolvedValue(null);
401+
mockBroker.call.mockResolvedValue({ name: 'my_obj' });
402+
403+
const result = await dispatcher.handleMetadata('/objects/my_obj', { request: {} }, 'GET');
404+
expect(result.handled).toBe(true);
405+
expect(mockBroker.call).toHaveBeenCalledWith(
406+
'metadata.getObject',
407+
{ objectName: 'my_obj' },
408+
{ request: {} }
409+
);
410+
});
411+
});
412+
});
413+
414+
// ═══════════════════════════════════════════════════════════════
415+
// Synchronous service resolution (backward compatibility)
416+
// ═══════════════════════════════════════════════════════════════
417+
418+
describe('Synchronous service resolution (backward compat)', () => {
419+
it('should work with synchronous service from services Map', async () => {
420+
const syncAnalytics = {
421+
query: vi.fn().mockResolvedValue({ rows: [], total: 0 }),
422+
};
423+
(kernel as any).services = new Map([['analytics', syncAnalytics]]);
424+
425+
const result = await dispatcher.handleAnalytics('query', 'POST', {}, { request: {} });
426+
expect(result.handled).toBe(true);
427+
expect(syncAnalytics.query).toHaveBeenCalled();
428+
});
429+
430+
it('should work with synchronous getService returning service directly', async () => {
431+
const syncAuto = {
432+
listFlows: vi.fn().mockResolvedValue(['flow_x']),
433+
};
434+
(kernel as any).getService = vi.fn().mockReturnValue(syncAuto);
435+
436+
const result = await dispatcher.handleAutomation('', 'GET', {}, { request: {} });
437+
expect(result.handled).toBe(true);
438+
expect(result.response?.body?.data?.flows).toEqual(['flow_x']);
439+
});
440+
});
441+
442+
// ═══════════════════════════════════════════════════════════════
443+
// Error handling for service method failures
444+
// ═══════════════════════════════════════════════════════════════
445+
446+
describe('Service method error handling', () => {
447+
it('should propagate analytics query error', async () => {
448+
const badAnalytics = {
449+
query: vi.fn().mockRejectedValue(new Error('Query timeout')),
450+
};
451+
(kernel as any).getService = vi.fn().mockResolvedValue(badAnalytics);
452+
453+
await expect(
454+
dispatcher.handleAnalytics('query', 'POST', {}, { request: {} })
455+
).rejects.toThrow('Query timeout');
456+
});
457+
458+
it('should propagate storage upload error', async () => {
459+
const badStorage = {
460+
upload: vi.fn().mockRejectedValue(new Error('Disk full')),
461+
};
462+
(kernel as any).getService = vi.fn().mockImplementation((name: string) => {
463+
if (name === 'file-storage') return Promise.resolve(badStorage);
464+
return null;
465+
});
466+
467+
await expect(
468+
dispatcher.handleStorage('/upload', 'POST', { data: 'file' }, { request: {} })
469+
).rejects.toThrow('Disk full');
470+
});
471+
});
207472
});

0 commit comments

Comments
 (0)