Skip to content

Commit fef6474

Browse files
NitayRabiNitayRabi
authored
Merge pull request #34 from code-rabi/fix/symbol-not-found-error-and-tif-type
fix(orders): preserve "Symbol not found" errors and narrow tif type (followup to #24)
2 parents 72f648a + 44c087a commit fef6474

4 files changed

Lines changed: 286 additions & 14 deletions

File tree

‎src/ib-client.ts‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,25 @@ interface OrderRequest {
2121
stopPrice?: number;
2222
suppressConfirmations?: boolean;
2323
exchange?: string;
24-
tif?: string;
24+
tif?: "DAY" | "GTC" | "IOC" | "OPG";
2525
}
2626

2727
const isError = (error: unknown): error is Error => {
2828
return error instanceof Error;
2929
};
3030

31+
/**
32+
* Thrown when a symbol (optionally scoped to an exchange) cannot be resolved
33+
* via `secdef/search`. Distinct error class so callers receive the specific
34+
* "Symbol ... not found" message instead of a swallowed generic one.
35+
*/
36+
export class SymbolNotFoundError extends Error {
37+
constructor(message: string) {
38+
super(message);
39+
this.name = "SymbolNotFoundError";
40+
}
41+
}
42+
3143
export class IBClient {
3244
private client!: AxiosInstance;
3345
private baseUrl!: string;
@@ -368,9 +380,9 @@ export class IBClient {
368380
searchUrl += `&name=${encodeURIComponent(exchange)}`;
369381
}
370382
const searchResponse = await this.client.get(searchUrl);
371-
383+
372384
if (!searchResponse.data || searchResponse.data.length === 0) {
373-
throw new Error(`Symbol ${symbol}${exchange ? ' on ' + exchange : ''} not found`);
385+
throw new SymbolNotFoundError(`Symbol ${symbol}${exchange ? ' on ' + exchange : ''} not found`);
374386
}
375387

376388
const contract = searchResponse.data[0];
@@ -391,14 +403,19 @@ export class IBClient {
391403
};
392404
} catch (error) {
393405
Logger.error("Failed to get market data:", error);
394-
406+
395407
// Check if this is likely an authentication error
396408
if (this.isAuthenticationError(error)) {
397409
const authError = new Error(`Authentication required to retrieve market data for ${symbol}. Please authenticate with Interactive Brokers first.`);
398410
(authError as any).isAuthError = true;
399411
throw authError;
400412
}
401-
413+
414+
// Preserve the specific "Symbol ... not found" message for callers
415+
if (error instanceof SymbolNotFoundError) {
416+
throw error;
417+
}
418+
402419
throw new Error(`Failed to retrieve market data for ${symbol}`);
403420
}
404421
}
@@ -436,9 +453,9 @@ export class IBClient {
436453
searchUrl += `&name=${encodeURIComponent(orderRequest.exchange)}`;
437454
}
438455
const searchResponse = await this.client.get(searchUrl);
439-
456+
440457
if (!searchResponse.data || searchResponse.data.length === 0) {
441-
throw new Error(`Symbol ${orderRequest.symbol}${orderRequest.exchange ? ' on ' + orderRequest.exchange : ''} not found`);
458+
throw new SymbolNotFoundError(`Symbol ${orderRequest.symbol}${orderRequest.exchange ? ' on ' + orderRequest.exchange : ''} not found`);
442459
}
443460

444461
const contract = searchResponse.data[0];
@@ -493,14 +510,19 @@ export class IBClient {
493510
return response.data;
494511
} catch (error) {
495512
Logger.error("Failed to place order:", error);
496-
513+
497514
// Check if this is likely an authentication error
498515
if (this.isAuthenticationError(error)) {
499516
const authError = new Error("Authentication required to place orders. Please authenticate with Interactive Brokers first.");
500517
(authError as any).isAuthError = true;
501518
throw authError;
502519
}
503-
520+
521+
// Preserve the specific "Symbol ... not found" message for callers
522+
if (error instanceof SymbolNotFoundError) {
523+
throw error;
524+
}
525+
504526
throw new Error("Failed to place order");
505527
}
506528
}

‎test/ib-client.test.ts‎

Lines changed: 185 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// test/ib-client.test.ts
22
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
3-
import { IBClient } from '../src/ib-client.js';
3+
import { IBClient, SymbolNotFoundError } from '../src/ib-client.js';
44
import axios from 'axios';
55

66
// Mock axios
@@ -176,12 +176,66 @@ describe('IBClient', () => {
176176

177177
it('should throw error if symbol not found', async () => {
178178
const mockClient = vi.mocked(axios.create).mock.results[0].value;
179-
179+
180180
// Mock empty search response
181181
mockClient.get.mockResolvedValueOnce({ data: [] });
182-
182+
183+
// The specific "Symbol ... not found" message should reach the caller
184+
// (not be swallowed by the generic "Failed to retrieve market data" catch)
183185
await expect(client.getMarketData('INVALID')).rejects.toThrow(
184-
'Failed to retrieve market data'
186+
'Symbol INVALID not found'
187+
);
188+
});
189+
190+
it('should propagate SymbolNotFoundError instance to callers', async () => {
191+
const mockClient = vi.mocked(axios.create).mock.results[0].value;
192+
193+
mockClient.get.mockResolvedValueOnce({ data: [] });
194+
195+
await expect(client.getMarketData('INVALID')).rejects.toBeInstanceOf(SymbolNotFoundError);
196+
});
197+
198+
it('should include exchange in secdef/search URL when provided', async () => {
199+
const mockClient = vi.mocked(axios.create).mock.results[0].value;
200+
201+
mockClient.get.mockResolvedValueOnce({
202+
data: [{ conid: 265598, symbol: 'AAPL' }],
203+
});
204+
mockClient.get.mockResolvedValueOnce({
205+
data: [{ conid: 265598, price: 150.25 }],
206+
});
207+
208+
await client.getMarketData('AAPL', 'NASDAQ');
209+
210+
expect(mockClient.get).toHaveBeenCalledWith(
211+
expect.stringContaining('/iserver/secdef/search?symbol=AAPL&name=NASDAQ')
212+
);
213+
});
214+
215+
it('should URL-encode the exchange parameter', async () => {
216+
const mockClient = vi.mocked(axios.create).mock.results[0].value;
217+
218+
mockClient.get.mockResolvedValueOnce({
219+
data: [{ conid: 265598, symbol: 'AAPL' }],
220+
});
221+
mockClient.get.mockResolvedValueOnce({
222+
data: [{ conid: 265598, price: 150.25 }],
223+
});
224+
225+
await client.getMarketData('AAPL', 'NYSE ARCA');
226+
227+
expect(mockClient.get).toHaveBeenCalledWith(
228+
expect.stringContaining('&name=NYSE%20ARCA')
229+
);
230+
});
231+
232+
it('should mention the exchange in the not-found error when provided', async () => {
233+
const mockClient = vi.mocked(axios.create).mock.results[0].value;
234+
235+
mockClient.get.mockResolvedValueOnce({ data: [] });
236+
237+
await expect(client.getMarketData('INVALID', 'NASDAQ')).rejects.toThrow(
238+
'Symbol INVALID on NASDAQ not found'
185239
);
186240
});
187241
});
@@ -260,6 +314,133 @@ describe('IBClient', () => {
260314
);
261315
});
262316

317+
it('should default tif to DAY when not specified', async () => {
318+
const mockClient = vi.mocked(axios.create).mock.results[0].value;
319+
320+
mockClient.get.mockResolvedValueOnce({
321+
data: [{ conid: 265598, symbol: 'AAPL' }],
322+
});
323+
mockClient.post.mockResolvedValueOnce({
324+
data: [{ id: 'order-123' }],
325+
});
326+
327+
await client.placeOrder({
328+
accountId: 'U12345',
329+
symbol: 'AAPL',
330+
action: 'BUY',
331+
orderType: 'MKT',
332+
quantity: 10,
333+
});
334+
335+
expect(mockClient.post).toHaveBeenCalledWith(
336+
expect.any(String),
337+
expect.objectContaining({
338+
orders: expect.arrayContaining([
339+
expect.objectContaining({ tif: 'DAY' }),
340+
]),
341+
})
342+
);
343+
});
344+
345+
it('should use the user-provided tif when given', async () => {
346+
const mockClient = vi.mocked(axios.create).mock.results[0].value;
347+
348+
mockClient.get.mockResolvedValueOnce({
349+
data: [{ conid: 265598, symbol: 'AAPL' }],
350+
});
351+
mockClient.post.mockResolvedValueOnce({
352+
data: [{ id: 'order-123' }],
353+
});
354+
355+
await client.placeOrder({
356+
accountId: 'U12345',
357+
symbol: 'AAPL',
358+
action: 'BUY',
359+
orderType: 'MKT',
360+
quantity: 10,
361+
tif: 'GTC',
362+
});
363+
364+
expect(mockClient.post).toHaveBeenCalledWith(
365+
expect.any(String),
366+
expect.objectContaining({
367+
orders: expect.arrayContaining([
368+
expect.objectContaining({ tif: 'GTC' }),
369+
]),
370+
})
371+
);
372+
});
373+
374+
it('should include exchange in secdef/search URL when provided', async () => {
375+
const mockClient = vi.mocked(axios.create).mock.results[0].value;
376+
377+
mockClient.get.mockResolvedValueOnce({
378+
data: [{ conid: 265598, symbol: 'AAPL' }],
379+
});
380+
mockClient.post.mockResolvedValueOnce({
381+
data: [{ id: 'order-123' }],
382+
});
383+
384+
await client.placeOrder({
385+
accountId: 'U12345',
386+
symbol: 'AAPL',
387+
action: 'BUY',
388+
orderType: 'MKT',
389+
quantity: 10,
390+
exchange: 'NASDAQ',
391+
});
392+
393+
expect(mockClient.get).toHaveBeenCalledWith(
394+
expect.stringContaining('/iserver/secdef/search?symbol=AAPL&name=NASDAQ')
395+
);
396+
});
397+
398+
it('should include exchange in the order payload when specified', async () => {
399+
const mockClient = vi.mocked(axios.create).mock.results[0].value;
400+
401+
mockClient.get.mockResolvedValueOnce({
402+
data: [{ conid: 265598, symbol: 'AAPL' }],
403+
});
404+
mockClient.post.mockResolvedValueOnce({
405+
data: [{ id: 'order-123' }],
406+
});
407+
408+
await client.placeOrder({
409+
accountId: 'U12345',
410+
symbol: 'AAPL',
411+
action: 'BUY',
412+
orderType: 'MKT',
413+
quantity: 10,
414+
exchange: 'NASDAQ',
415+
});
416+
417+
expect(mockClient.post).toHaveBeenCalledWith(
418+
expect.any(String),
419+
expect.objectContaining({
420+
orders: expect.arrayContaining([
421+
expect.objectContaining({ exchange: 'NASDAQ' }),
422+
]),
423+
})
424+
);
425+
});
426+
427+
it('should propagate SymbolNotFoundError when symbol cannot be resolved', async () => {
428+
const mockClient = vi.mocked(axios.create).mock.results[0].value;
429+
430+
// Mock empty search response — no matching symbol
431+
mockClient.get.mockResolvedValueOnce({ data: [] });
432+
433+
await expect(
434+
client.placeOrder({
435+
accountId: 'U12345',
436+
symbol: 'INVALID',
437+
action: 'BUY',
438+
orderType: 'MKT',
439+
quantity: 10,
440+
})
441+
).rejects.toThrow('Symbol INVALID not found');
442+
});
443+
263444
it('should include stopPrice for stop orders', async () => {
264445
const mockClient = vi.mocked(axios.create).mock.results[0].value;
265446

‎test/tool-definitions.test.ts‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,52 @@ describe('Tool Definitions - Zod Schemas', () => {
164164
quantity: 10,
165165
suppressConfirmations: true,
166166
};
167-
167+
168+
const result = PlaceOrderZodSchema.safeParse(validOrder);
169+
expect(result.success).toBe(true);
170+
});
171+
172+
it.each(['DAY', 'GTC', 'IOC', 'OPG'] as const)(
173+
'should accept tif value %s',
174+
(tif) => {
175+
const validOrder = {
176+
accountId: 'U12345',
177+
symbol: 'AAPL',
178+
action: 'BUY' as const,
179+
orderType: 'MKT' as const,
180+
quantity: 10,
181+
tif,
182+
};
183+
184+
const result = PlaceOrderZodSchema.safeParse(validOrder);
185+
expect(result.success).toBe(true);
186+
}
187+
);
188+
189+
it('should reject an invalid tif value', () => {
190+
const invalidOrder = {
191+
accountId: 'U12345',
192+
symbol: 'AAPL',
193+
action: 'BUY' as const,
194+
orderType: 'MKT' as const,
195+
quantity: 10,
196+
tif: 'BAD',
197+
};
198+
199+
const result = PlaceOrderZodSchema.safeParse(invalidOrder);
200+
expect(result.success).toBe(false);
201+
});
202+
203+
it('should accept exchange when provided alongside required fields', () => {
204+
const validOrder = {
205+
accountId: 'U12345',
206+
symbol: 'AAPL',
207+
action: 'BUY' as const,
208+
orderType: 'MKT' as const,
209+
quantity: 10,
210+
exchange: 'NASDAQ',
211+
};
212+
168213
const result = PlaceOrderZodSchema.safeParse(validOrder);
169214
expect(result.success).toBe(true);
170215
});

‎test/tool-handlers.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,30 @@ describe('ToolHandlers', () => {
173173
);
174174
});
175175

176+
it('should forward exchange and tif to ibClient.placeOrder when provided', async () => {
177+
const mockResponse = { orderId: '123', status: 'Submitted' };
178+
mockIBClient.placeOrder = vi.fn().mockResolvedValue(mockResponse);
179+
180+
const orderInput = {
181+
accountId: 'U12345',
182+
symbol: 'AAPL',
183+
action: 'BUY' as const,
184+
orderType: 'MKT' as const,
185+
quantity: 10,
186+
exchange: 'NASDAQ',
187+
tif: 'GTC' as const,
188+
};
189+
190+
await handlers.placeOrder(orderInput);
191+
192+
expect(mockIBClient.placeOrder).toHaveBeenCalledWith(
193+
expect.objectContaining({
194+
exchange: 'NASDAQ',
195+
tif: 'GTC',
196+
})
197+
);
198+
});
199+
176200
it('should handle order placement errors', async () => {
177201
mockIBClient.placeOrder = vi.fn().mockRejectedValue(new Error('Order failed'));
178202

0 commit comments

Comments
 (0)