From 12054766486bfc38b214a894c80ae5f474e7537f Mon Sep 17 00:00:00 2001 From: Alexandre Catarino Date: Fri, 24 Jul 2026 18:44:57 +0100 Subject: [PATCH 01/13] Improve unknown Tradier order id error message Explain that unknown order ids are usually caused by manual orders placed directly on the brokerage account, why LEAN terminates the algorithm, and how to trade manually safely (QuantConnect IDE/LEAN CLI or stop-and-redeploy). Co-Authored-By: Claude Fable 5 --- QuantConnect.TradierBrokerage/TradierBrokerage.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/QuantConnect.TradierBrokerage/TradierBrokerage.cs b/QuantConnect.TradierBrokerage/TradierBrokerage.cs index 6e88c72..79774ea 100644 --- a/QuantConnect.TradierBrokerage/TradierBrokerage.cs +++ b/QuantConnect.TradierBrokerage/TradierBrokerage.cs @@ -1288,7 +1288,10 @@ private void CheckForFills() // if we still have unknown IDs then we've gotta bail on the algorithm var ids = string.Join(", ", stillUnknownOrderIDs); Log.Error("TradierBrokerage.CheckForFills(): Unable to verify all missing brokerage IDs: " + ids); - OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Error, "UnknownOrderId", "Received unknown Tradier order id(s): " + ids)); + OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Error, "UnknownOrderId", "Received unknown Tradier order id(s): " + ids + + ". These orders were likely placed manually on the account. LEAN terminates live algorithms when it detects interference outside of the algorithm's control" + + " to avoid race conditions between the account owner and the algorithm, so avoid placing manual orders while the algorithm is running." + + " To adjust your holdings manually, place your trades through the QuantConnect IDE or LEAN CLI, or stop the algorithm, place your trades, and then redeploy.")); return; } } From 2aa10d13fb15753c3a6195ddb164020743ee7c4b Mon Sep 17 00:00:00 2001 From: Alexandre Catarino Date: Thu, 30 Jul 2026 18:38:36 +0100 Subject: [PATCH 02/13] Support orders placed outside of the algorithm Orders placed directly in the Tradier account are now offered to the algorithm through the NewBrokerageOrderNotification event, like the other brokerages do. A custom IBrokerageMessageHandler whose HandleOrder returns true takes ownership of them: LEAN assigns the order id and we start tracking the order, emitting its submission, fills and final status. The default handler ignores them, so unknown order ids keep terminating the algorithm as before, with the message now pointing at the handler. Rejected orders are skipped altogether instead of only within a one minute window: they never reached the account, so there is nothing to track. An empty response from the intraday order fetch no longer fails the algorithm for ids it could not inspect. They are left unverified so the next poll rechecks them, which is also why the error path no longer adds the ids back to the pending set: doing so blocked the recheck task from ever firing again. Co-Authored-By: Claude Opus 5 (1M context) --- .../TradierBrokerageAditionalTests.cs | 140 ++++++++++++++++++ .../TradierBrokerage.cs | 112 ++++++++++++-- 2 files changed, 243 insertions(+), 9 deletions(-) diff --git a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs index ca84868..ba427db 100644 --- a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs +++ b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs @@ -25,6 +25,7 @@ using QuantConnect.Util; using RestSharp; using System; +using System.Collections; using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; @@ -39,6 +40,9 @@ namespace QuantConnect.Tests.Brokerages.Tradier [TestFixture] public class TradierBrokerageAditionalTests { + private const long BrokerageSideOrderId = 1234; + private static readonly string[] BrokerageSideOrderBrokerIds = [BrokerageSideOrderId.ToStringInvariant()]; + [Test] public void InitializesFactoryFromComposer() { @@ -340,6 +344,142 @@ public void ReplacesProxyHtmlPageWithActionableMessageAfterRetriesAreExhausted(s restClient.Verify(x => x.Execute(It.IsAny()), Times.Exactly(2)); } + // Orders placed directly in the Tradier account, outside of the algorithm, are offered to the algorithm through + // the NewBrokerageOrderNotification event, so a brokerage message handler can take ownership of them instead of + // the algorithm being terminated for interference it doesn't control + [Test] + public void NotifiesOrdersPlacedOutsideOfTheAlgorithm() + { + var orderProvider = new OrderProvider(); + var brokerage = CreateBrokerageWithOrderTracking(orderProvider); + + var orderEvents = new List(); + brokerage.OrdersStatusChanged += (_, events) => orderEvents.AddRange(events); + + Order notifiedOrder = null; + brokerage.NewBrokerageOrderNotification += (_, e) => + { + notifiedOrder = e.Order; + // this is what the transaction handler does when the algorithm accepts the order: it assigns the Lean id + orderProvider.Add(e.Order); + }; + + Assert.IsTrue(InvokeTryHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m))); + + Assert.IsNotNull(notifiedOrder); + Assert.AreEqual(OrderType.Market, notifiedOrder.Type); + Assert.AreEqual("SPY", notifiedOrder.Symbol.Value); + Assert.AreEqual(10m, notifiedOrder.Quantity); + CollectionAssert.AreEqual(BrokerageSideOrderBrokerIds, notifiedOrder.BrokerId); + + // the order is first reported as submitted and then the fill it already had when we found it is emitted + Assert.AreEqual(2, orderEvents.Count); + Assert.AreEqual(OrderStatus.Submitted, orderEvents[0].Status); + Assert.AreEqual(OrderStatus.Filled, orderEvents[1].Status); + Assert.AreEqual(10m, orderEvents[1].FillQuantity); + Assert.AreEqual(123.45m, orderEvents[1].FillPrice); + } + + [Test] + public void TracksOpenOrdersPlacedOutsideOfTheAlgorithmForFutureFills() + { + var orderProvider = new OrderProvider(); + var brokerage = CreateBrokerageWithOrderTracking(orderProvider); + + var orderEvents = new List(); + brokerage.OrdersStatusChanged += (_, events) => orderEvents.AddRange(events); + brokerage.NewBrokerageOrderNotification += (_, e) => orderProvider.Add(e.Order); + + Assert.IsTrue(InvokeTryHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Open))); + + // the order is still open, so it's only reported as submitted, no fill event yet + Assert.AreEqual(1, orderEvents.Count); + Assert.AreEqual(OrderStatus.Submitted, orderEvents[0].Status); + + // and it's cached so the regular fill detection picks up its fills from now on + Assert.IsTrue(GetCachedOpenOrders(brokerage).Contains(BrokerageSideOrderId)); + } + + // When the order is not accepted, which is what the default brokerage message handler does, the order is left + // untracked and reported back as unhandled so the caller can fail the algorithm like it did before + [Test] + public void DoesNotTrackOrdersPlacedOutsideOfTheAlgorithmWhenTheyAreNotAccepted() + { + var brokerage = CreateBrokerageWithOrderTracking(new OrderProvider()); + + var orderEvents = new List(); + brokerage.OrdersStatusChanged += (_, events) => orderEvents.AddRange(events); + + var notified = false; + // the default brokerage message handler ignores these orders, leaving the Lean order id unset + brokerage.NewBrokerageOrderNotification += (_, e) => notified = true; + + Assert.IsFalse(InvokeTryHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m))); + + Assert.IsTrue(notified); + Assert.IsEmpty(orderEvents); + Assert.IsFalse(GetCachedOpenOrders(brokerage).Contains(BrokerageSideOrderId)); + } + + private static TradierOrder CreateBrokerageSideOrder(TradierOrderStatus status, decimal quantityExecuted = 0m) + { + return new TradierOrder + { + Id = BrokerageSideOrderId, + Type = TradierOrderType.Market, + Symbol = "SPY", + Direction = TradierOrderDirection.Buy, + Quantity = 10m, + Status = status, + Duration = TradierOrderDuration.Day, + QuantityExecuted = quantityExecuted, + RemainingQuantity = 10m - quantityExecuted, + LastFillPrice = quantityExecuted > 0 ? 123.45m : 0m, + AverageFillPrice = quantityExecuted > 0 ? 123.45m : 0m, + Class = TradierOrderClass.Equity, + TransactionDate = DateTime.UtcNow, + CreatedDate = DateTime.UtcNow + }; + } + + // Builds a brokerage with just the state the brokerage side order handling needs, skipping the heavy Initialize + // (license validation, timers, streaming threads) that a full construction would trigger. + private static TradierBrokerage CreateBrokerageWithOrderTracking(OrderProvider orderProvider) + { + var brokerage = new TradierBrokerage(); + + SetPrivateField(typeof(TradierBrokerage), brokerage, "_orderProvider", orderProvider); + SetPrivateField(typeof(TradierBrokerage), brokerage, "_securityProvider", new SecurityProvider()); + SetPrivateField(typeof(TradierBrokerage), brokerage, "_symbolMapper", new TradierSymbolMapper(_ => null)); + + // the cached open orders are keyed by a private type, so let reflection create the dictionary for us + var cachedOpenOrders = typeof(TradierBrokerage).GetField("_cachedOpenOrdersByTradierOrderID", + BindingFlags.NonPublic | BindingFlags.Instance); + cachedOpenOrders.SetValue(brokerage, Activator.CreateInstance(cachedOpenOrders.FieldType)); + + return brokerage; + } + + private static IDictionary GetCachedOpenOrders(TradierBrokerage brokerage) + { + return (IDictionary)typeof(TradierBrokerage) + .GetField("_cachedOpenOrdersByTradierOrderID", BindingFlags.NonPublic | BindingFlags.Instance) + .GetValue(brokerage); + } + + private static bool InvokeTryHandleBrokerageSideOrder(TradierBrokerage brokerage, TradierOrder brokerageSideOrder) + { + var method = typeof(TradierBrokerage).GetMethod("TryHandleBrokerageSideOrder", BindingFlags.NonPublic | BindingFlags.Instance); + try + { + return (bool)method.Invoke(brokerage, [brokerageSideOrder]); + } + catch (TargetInvocationException e) + { + throw e.InnerException; + } + } + private static IRestResponse CreateResponse(string content, HttpStatusCode statusCode = HttpStatusCode.OK) { return new RestResponse diff --git a/QuantConnect.TradierBrokerage/TradierBrokerage.cs b/QuantConnect.TradierBrokerage/TradierBrokerage.cs index 79774ea..19e990b 100644 --- a/QuantConnect.TradierBrokerage/TradierBrokerage.cs +++ b/QuantConnect.TradierBrokerage/TradierBrokerage.cs @@ -1276,22 +1276,44 @@ private void CheckForFills() // so don't treat orders placed externally in the account as an error if (_orderProvider != null && stillUnknownOrderIDs.Count > 0) { - // fetch all rejected intraday orders within the last minute, we're going to exclude rejected orders from the error condition - var recentOrders = GetIntradayAndPendingOrders().Where(x => x.Status == TradierOrderStatus.Rejected) - .Where(x => DateTime.UtcNow - x.TransactionDate < TimeSpan.FromMinutes(1)).ToHashSet(x => x.Id); + var intradayOrders = GetIntradayAndPendingOrders().ToDictionary(x => x.Id); + if (intradayOrders.Count == 0) + { + // the request failed and already reported it, leave these ids unverified so we recheck them + Log.Trace("TradierBrokerage.CheckForFills(): No orders returned, will recheck the missing brokerage IDs: " + + string.Join(", ", stillUnknownOrderIDs)); + return; + } - // remove recently rejected orders, sometimes we'll get updates for these but we've already marked them as rejected - stillUnknownOrderIDs.RemoveAll(x => recentOrders.Contains(x)); + stillUnknownOrderIDs.RemoveAll(x => + { + if (!intradayOrders.TryGetValue(x, out var unknownOrder)) + { + // we can't inspect the order, so we won't dismiss it + return false; + } + + // rejected orders never made it into the account, there's nothing to track + if (unknownOrder.Status == TradierOrderStatus.Rejected) + { + return true; + } + + // the order was placed outside of the algorithm, let it decide whether to take ownership of it + return TryHandleBrokerageSideOrder(unknownOrder); + }); if (stillUnknownOrderIDs.Count > 0) { - // if we still have unknown IDs then we've gotta bail on the algorithm + // the algorithm didn't take ownership of these orders, so we've gotta bail on it var ids = string.Join(", ", stillUnknownOrderIDs); Log.Error("TradierBrokerage.CheckForFills(): Unable to verify all missing brokerage IDs: " + ids); OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Error, "UnknownOrderId", "Received unknown Tradier order id(s): " + ids + ". These orders were likely placed manually on the account. LEAN terminates live algorithms when it detects interference outside of the algorithm's control" + " to avoid race conditions between the account owner and the algorithm, so avoid placing manual orders while the algorithm is running." + - " To adjust your holdings manually, place your trades through the QuantConnect IDE or LEAN CLI, or stop the algorithm, place your trades, and then redeploy.")); + " To adjust your holdings manually, place your trades through the QuantConnect IDE or LEAN CLI, or stop the algorithm, place your trades, and then redeploy." + + " If instead you want the algorithm to take ownership of the orders placed outside of it, set a custom brokerage message handler with" + + " 'SetBrokerageMessageHandler(...)' implementing a 'HandleOrder' method that returns true for the orders the algorithm should track.")); return; } } @@ -1304,8 +1326,8 @@ private void CheckForFills() } catch (Exception err) { - // we need to recheck these order ids since we failed, so add them back to the set - foreach (var id in localUnknownTradierOrderIDs) _unknownTradierOrderIDs.Add(id); + // we need to recheck these order ids since we failed, leaving them unverified is enough for the next + // poll to fire a new task for them, adding them back to the set would block that task from firing Log.Error(err); OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Warning, "UnknownIdResolution", "An error occurred while trying to resolve unknown Tradier order IDs: " + err)); @@ -1334,6 +1356,78 @@ private bool IsUnknownOrderID(KeyValuePair x) && !_filledTradierOrderIDs.Contains(x.Key); } + /// + /// Handles an order that was placed directly in the brokerage account, outside of the algorithm. + /// The algorithm's brokerage message handler decides whether it takes ownership of the order, if it does + /// we start tracking it so we emit its order events just like we do for the orders we placed ourselves. + /// + /// The Tradier order the algorithm is unaware of + /// True if the algorithm took ownership of the order and we started tracking it, false otherwise + private bool TryHandleBrokerageSideOrder(TradierOrder brokerageSideOrder) + { + Order leanOrder; + try + { + leanOrder = ConvertOrder(brokerageSideOrder); + } + catch (Exception err) + { + Log.Error(err, $"TradierBrokerage.TryHandleBrokerageSideOrder(): failed to convert Tradier order {brokerageSideOrder.Id}"); + return false; + } + + // the transaction handler assigns the Lean order id and marks the order as submitted when it accepts it + leanOrder.Status = OrderStatus.New; + OnNewBrokerageOrderNotification(new NewBrokerageOrderNotificationEventArgs(leanOrder)); + + if (leanOrder.Id == 0) + { + // the algorithm's brokerage message handler didn't accept the order, so we won't track it + return false; + } + + OnOrderEvent(new OrderEvent(leanOrder, DateTime.UtcNow, OrderFee.Zero, "Order was submitted outside of the algorithm") + { Status = OrderStatus.Submitted }); + + // start tracking the order from a state with no execution progress, this way the regular fill detection + // emits the fills and the final status the order might already have + var cachedOrder = new TradierCachedOpenOrder(new TradierOrder + { + Id = brokerageSideOrder.Id, + Type = brokerageSideOrder.Type, + Symbol = brokerageSideOrder.Symbol, + OptionSymbol = brokerageSideOrder.OptionSymbol, + Direction = brokerageSideOrder.Direction, + Quantity = brokerageSideOrder.Quantity, + // we just reported the order as submitted, keeping the status of orders that are still open avoids + // emitting an event for a status change that never happened + Status = OrderIsOpen(brokerageSideOrder) ? brokerageSideOrder.Status : TradierOrderStatus.Submitted, + Duration = brokerageSideOrder.Duration, + Price = brokerageSideOrder.Price, + AverageFillPrice = 0m, + QuantityExecuted = 0m, + LastFillPrice = 0m, + LastFillQuantity = 0m, + RemainingQuantity = brokerageSideOrder.Quantity, + CreatedDate = brokerageSideOrder.CreatedDate, + TransactionDate = brokerageSideOrder.TransactionDate, + Class = brokerageSideOrder.Class, + NumberOfLegs = brokerageSideOrder.NumberOfLegs, + Legs = brokerageSideOrder.Legs + }); + _cachedOpenOrdersByTradierOrderID[brokerageSideOrder.Id] = cachedOrder; + + ProcessPotentiallyUpdatedOrder(cachedOrder, brokerageSideOrder); + + // if the order is still open, update the cached value + if (!OrderIsClosed(brokerageSideOrder)) + { + UpdateCachedOpenOrder(brokerageSideOrder.Id, brokerageSideOrder); + } + + return true; + } + private void ProcessPotentiallyUpdatedOrder(TradierCachedOpenOrder cachedOrder, TradierOrder updatedOrder) { // check for fills or status changes, for either fire a fill event From 57fdb7c8d796d936fdd5dfe09ceea435d72ca99a Mon Sep 17 00:00:00 2001 From: Alexandre Catarino Date: Thu, 30 Jul 2026 18:48:18 +0100 Subject: [PATCH 03/13] Report orders that couldn't be offered to the algorithm apart An order we couldn't fetch the details of, or couldn't convert into a Lean order, was reported with the same message as an order the algorithm was offered and declined. That message tells the user to set a brokerage message handler whose HandleOrder returns true, which doesn't help when the order never reached the handler in the first place. Both outcomes are now distinguished and get their own message. Co-Authored-By: Claude Opus 5 (1M context) --- .../TradierBrokerageAditionalTests.cs | 41 +++++++--- .../TradierBrokerage.cs | 76 +++++++++++++++---- 2 files changed, 93 insertions(+), 24 deletions(-) diff --git a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs index ba427db..5b638de 100644 --- a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs +++ b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs @@ -364,7 +364,7 @@ public void NotifiesOrdersPlacedOutsideOfTheAlgorithm() orderProvider.Add(e.Order); }; - Assert.IsTrue(InvokeTryHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m))); + Assert.AreEqual("Tracked", InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m))); Assert.IsNotNull(notifiedOrder); Assert.AreEqual(OrderType.Market, notifiedOrder.Type); @@ -390,7 +390,7 @@ public void TracksOpenOrdersPlacedOutsideOfTheAlgorithmForFutureFills() brokerage.OrdersStatusChanged += (_, events) => orderEvents.AddRange(events); brokerage.NewBrokerageOrderNotification += (_, e) => orderProvider.Add(e.Order); - Assert.IsTrue(InvokeTryHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Open))); + Assert.AreEqual("Tracked", InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Open))); // the order is still open, so it's only reported as submitted, no fill event yet Assert.AreEqual(1, orderEvents.Count); @@ -401,7 +401,7 @@ public void TracksOpenOrdersPlacedOutsideOfTheAlgorithmForFutureFills() } // When the order is not accepted, which is what the default brokerage message handler does, the order is left - // untracked and reported back as unhandled so the caller can fail the algorithm like it did before + // untracked and reported back as not accepted so the caller can fail the algorithm like it did before [Test] public void DoesNotTrackOrdersPlacedOutsideOfTheAlgorithmWhenTheyAreNotAccepted() { @@ -414,19 +414,41 @@ public void DoesNotTrackOrdersPlacedOutsideOfTheAlgorithmWhenTheyAreNotAccepted( // the default brokerage message handler ignores these orders, leaving the Lean order id unset brokerage.NewBrokerageOrderNotification += (_, e) => notified = true; - Assert.IsFalse(InvokeTryHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m))); + Assert.AreEqual("NotAccepted", InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m))); Assert.IsTrue(notified); Assert.IsEmpty(orderEvents); Assert.IsFalse(GetCachedOpenOrders(brokerage).Contains(BrokerageSideOrderId)); } - private static TradierOrder CreateBrokerageSideOrder(TradierOrderStatus status, decimal quantityExecuted = 0m) + // Orders LEAN cannot represent, like the multileg only order types, are never offered to the algorithm, so they + // are reported apart from the ones it declined: telling the user to accept them in a handler wouldn't help + [Test] + public void ReportsOrdersItCannotConvertAsUnprocessable() + { + var brokerage = CreateBrokerageWithOrderTracking(new OrderProvider()); + + var orderEvents = new List(); + brokerage.OrdersStatusChanged += (_, events) => orderEvents.AddRange(events); + + var notified = false; + brokerage.NewBrokerageOrderNotification += (_, e) => notified = true; + + var brokerageSideOrder = CreateBrokerageSideOrder(TradierOrderStatus.Open, type: TradierOrderType.Credit); + Assert.AreEqual("Unprocessable", InvokeHandleBrokerageSideOrder(brokerage, brokerageSideOrder)); + + Assert.IsFalse(notified); + Assert.IsEmpty(orderEvents); + Assert.IsFalse(GetCachedOpenOrders(brokerage).Contains(BrokerageSideOrderId)); + } + + private static TradierOrder CreateBrokerageSideOrder(TradierOrderStatus status, decimal quantityExecuted = 0m, + TradierOrderType type = TradierOrderType.Market) { return new TradierOrder { Id = BrokerageSideOrderId, - Type = TradierOrderType.Market, + Type = type, Symbol = "SPY", Direction = TradierOrderDirection.Buy, Quantity = 10m, @@ -467,12 +489,13 @@ private static IDictionary GetCachedOpenOrders(TradierBrokerage brokerage) .GetValue(brokerage); } - private static bool InvokeTryHandleBrokerageSideOrder(TradierBrokerage brokerage, TradierOrder brokerageSideOrder) + // the result is a private enum, so it's compared by name + private static string InvokeHandleBrokerageSideOrder(TradierBrokerage brokerage, TradierOrder brokerageSideOrder) { - var method = typeof(TradierBrokerage).GetMethod("TryHandleBrokerageSideOrder", BindingFlags.NonPublic | BindingFlags.Instance); + var method = typeof(TradierBrokerage).GetMethod("HandleBrokerageSideOrder", BindingFlags.NonPublic | BindingFlags.Instance); try { - return (bool)method.Invoke(brokerage, [brokerageSideOrder]); + return method.Invoke(brokerage, [brokerageSideOrder]).ToString(); } catch (TargetInvocationException e) { diff --git a/QuantConnect.TradierBrokerage/TradierBrokerage.cs b/QuantConnect.TradierBrokerage/TradierBrokerage.cs index 19e990b..24cb780 100644 --- a/QuantConnect.TradierBrokerage/TradierBrokerage.cs +++ b/QuantConnect.TradierBrokerage/TradierBrokerage.cs @@ -1285,28 +1285,53 @@ private void CheckForFills() return; } - stillUnknownOrderIDs.RemoveAll(x => + // orders the algorithm was offered and didn't accept, and orders we couldn't offer it at all + var !intradayOrders.TryGetValue(x, out var unknownOrder) = new List(); + var unprocessableOrderIDs = new List(); + foreach (var stillUnknownOrderID in stillUnknownOrderIDs) { - if (!intradayOrders.TryGetValue(x, out var unknownOrder)) + if (!intradayOrders.TryGetValue(stillUnknownOrderID, out var unknownOrder)) { - // we can't inspect the order, so we won't dismiss it - return false; + // we don't have the details of the order, so we can't offer it to the algorithm + unprocessableOrderIDs.Add(stillUnknownOrderID); + continue; } // rejected orders never made it into the account, there's nothing to track if (unknownOrder.Status == TradierOrderStatus.Rejected) { - return true; + continue; } // the order was placed outside of the algorithm, let it decide whether to take ownership of it - return TryHandleBrokerageSideOrder(unknownOrder); - }); + switch (HandleBrokerageSideOrder(unknownOrder)) + { + case BrokerageSideOrderResult.NotAccepted: + notAcceptedOrderIDs.Add(stillUnknownOrderID); + break; + + case BrokerageSideOrderResult.Unprocessable: + unprocessableOrderIDs.Add(stillUnknownOrderID); + break; + } + } + + if (unprocessableOrderIDs.Count > 0) + { + // we couldn't hand these orders over to the algorithm, so we've gotta bail on it + var ids = string.Join(", ", unprocessableOrderIDs); + Log.Error("TradierBrokerage.CheckForFills(): Unable to process the missing brokerage IDs: " + ids); + OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Error, "UnprocessableOrderId", "Unable to process the Tradier order id(s): " + ids + + ". Their details could not be fetched from Tradier or they cannot be represented as LEAN orders, so the algorithm cannot track them." + + " LEAN terminates live algorithms when it detects interference outside of the algorithm's control to avoid race conditions between" + + " the account owner and the algorithm, so avoid placing manual orders while the algorithm is running.")); + return; + } - if (stillUnknownOrderIDs.Count > 0) + if (notAcceptedOrderIDs.Count > 0) { // the algorithm didn't take ownership of these orders, so we've gotta bail on it - var ids = string.Join(", ", stillUnknownOrderIDs); + var ids = string.Join(", ", notAcceptedOrderIDs); Log.Error("TradierBrokerage.CheckForFills(): Unable to verify all missing brokerage IDs: " + ids); OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Error, "UnknownOrderId", "Received unknown Tradier order id(s): " + ids + ". These orders were likely placed manually on the account. LEAN terminates live algorithms when it detects interference outside of the algorithm's control" + @@ -1362,8 +1387,8 @@ private bool IsUnknownOrderID(KeyValuePair x) /// we start tracking it so we emit its order events just like we do for the orders we placed ourselves. /// /// The Tradier order the algorithm is unaware of - /// True if the algorithm took ownership of the order and we started tracking it, false otherwise - private bool TryHandleBrokerageSideOrder(TradierOrder brokerageSideOrder) + /// The outcome of offering the order to the algorithm + private BrokerageSideOrderResult HandleBrokerageSideOrder(TradierOrder brokerageSideOrder) { Order leanOrder; try @@ -1372,8 +1397,8 @@ private bool TryHandleBrokerageSideOrder(TradierOrder brokerageSideOrder) } catch (Exception err) { - Log.Error(err, $"TradierBrokerage.TryHandleBrokerageSideOrder(): failed to convert Tradier order {brokerageSideOrder.Id}"); - return false; + Log.Error(err, $"TradierBrokerage.HandleBrokerageSideOrder(): failed to convert Tradier order {brokerageSideOrder.Id}"); + return BrokerageSideOrderResult.Unprocessable; } // the transaction handler assigns the Lean order id and marks the order as submitted when it accepts it @@ -1383,7 +1408,7 @@ private bool TryHandleBrokerageSideOrder(TradierOrder brokerageSideOrder) if (leanOrder.Id == 0) { // the algorithm's brokerage message handler didn't accept the order, so we won't track it - return false; + return BrokerageSideOrderResult.NotAccepted; } OnOrderEvent(new OrderEvent(leanOrder, DateTime.UtcNow, OrderFee.Zero, "Order was submitted outside of the algorithm") @@ -1425,7 +1450,7 @@ private bool TryHandleBrokerageSideOrder(TradierOrder brokerageSideOrder) UpdateCachedOpenOrder(brokerageSideOrder.Id, brokerageSideOrder); } - return true; + return BrokerageSideOrderResult.Tracked; } private void ProcessPotentiallyUpdatedOrder(TradierCachedOpenOrder cachedOrder, TradierOrder updatedOrder) @@ -2057,6 +2082,27 @@ public TradierPlaceOrderRequest Next() } } + /// + /// The outcome of offering an order placed outside of the algorithm to it + /// + private enum BrokerageSideOrderResult + { + /// + /// The algorithm took ownership of the order and we started tracking it + /// + Tracked, + + /// + /// The algorithm was offered the order and didn't accept it + /// + NotAccepted, + + /// + /// The order couldn't be offered to the algorithm, we failed to convert it into a Lean order + /// + Unprocessable + } + private class TradierCachedOpenOrder { public bool EmittedOrderFee; From b2696a847084494fde506e12eacc33256bca52ab Mon Sep 17 00:00:00 2001 From: Alexandre Catarino Date: Thu, 30 Jul 2026 18:52:45 +0100 Subject: [PATCH 04/13] Fix mangled declaration and dedup the reported order ids The notAcceptedOrderIDs declaration was corrupted in the previous commit, leaving the project uncompilable. Restore it, and use hash sets for both collections so an order id can never be reported twice. Co-Authored-By: Claude Opus 5 (1M context) --- QuantConnect.TradierBrokerage/TradierBrokerage.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/QuantConnect.TradierBrokerage/TradierBrokerage.cs b/QuantConnect.TradierBrokerage/TradierBrokerage.cs index 24cb780..78667c5 100644 --- a/QuantConnect.TradierBrokerage/TradierBrokerage.cs +++ b/QuantConnect.TradierBrokerage/TradierBrokerage.cs @@ -1286,8 +1286,8 @@ private void CheckForFills() } // orders the algorithm was offered and didn't accept, and orders we couldn't offer it at all - var !intradayOrders.TryGetValue(x, out var unknownOrder) = new List(); - var unprocessableOrderIDs = new List(); + var notAcceptedOrderIDs = new HashSet(); + var unprocessableOrderIDs = new HashSet(); foreach (var stillUnknownOrderID in stillUnknownOrderIDs) { if (!intradayOrders.TryGetValue(stillUnknownOrderID, out var unknownOrder)) From 55f40a73070ef84666bc76de601c85ba5370dc53 Mon Sep 17 00:00:00 2001 From: Alexandre Catarino Date: Thu, 30 Jul 2026 19:03:21 +0100 Subject: [PATCH 05/13] Only skip rejections that can be ours Restores the one minute window on the rejected order check: the fill polling runs every couple of seconds, so a rejection older than that is not one of ours whose brokerage id we failed to resolve, it was placed outside of the algorithm and should be offered to it like any other order. The orders are now kept in a dictionary rather than a set of ids, so the same lookup serves both this check and the brokerage side order handling. Co-Authored-By: Claude Opus 5 (1M context) --- QuantConnect.TradierBrokerage/TradierBrokerage.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/QuantConnect.TradierBrokerage/TradierBrokerage.cs b/QuantConnect.TradierBrokerage/TradierBrokerage.cs index 78667c5..7a8cc5d 100644 --- a/QuantConnect.TradierBrokerage/TradierBrokerage.cs +++ b/QuantConnect.TradierBrokerage/TradierBrokerage.cs @@ -1297,8 +1297,11 @@ private void CheckForFills() continue; } - // rejected orders never made it into the account, there's nothing to track - if (unknownOrder.Status == TradierOrderStatus.Rejected) + // skip recently rejected orders, sometimes we'll get updates for these but we've + // already marked them as rejected. We pull the orders every couple of seconds, so + // an older rejection is not one of ours, it was placed outside of the algorithm + if (unknownOrder.Status == TradierOrderStatus.Rejected + && DateTime.UtcNow - unknownOrder.TransactionDate < TimeSpan.FromMinutes(1)) { continue; } From f2b1b215bf562a82c30f21de8ad6769054309a8e Mon Sep 17 00:00:00 2001 From: Alexandre Catarino Date: Thu, 30 Jul 2026 19:29:08 +0100 Subject: [PATCH 06/13] Keep the original recently rejected orders filter Restores the original recentOrders lines on top of a single fetch, instead of testing the status and transaction date inside the loop, keeping the diff closer to what the method looked like. Co-Authored-By: Claude Opus 5 (1M context) --- .../TradierBrokerage.cs | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/QuantConnect.TradierBrokerage/TradierBrokerage.cs b/QuantConnect.TradierBrokerage/TradierBrokerage.cs index 7a8cc5d..9584014 100644 --- a/QuantConnect.TradierBrokerage/TradierBrokerage.cs +++ b/QuantConnect.TradierBrokerage/TradierBrokerage.cs @@ -1276,7 +1276,7 @@ private void CheckForFills() // so don't treat orders placed externally in the account as an error if (_orderProvider != null && stillUnknownOrderIDs.Count > 0) { - var intradayOrders = GetIntradayAndPendingOrders().ToDictionary(x => x.Id); + var intradayOrders = GetIntradayAndPendingOrders(); if (intradayOrders.Count == 0) { // the request failed and already reported it, leave these ids unverified so we recheck them @@ -1285,28 +1285,29 @@ private void CheckForFills() return; } - // orders the algorithm was offered and didn't accept, and orders we couldn't offer it at all + // fetch all rejected intraday orders within the last minute, we're going to exclude rejected orders from the error + // condition. We pull the orders every couple of seconds, so an older rejection can't be one of ours + var recentOrders = intradayOrders.Where(x => x.Status == TradierOrderStatus.Rejected) + .Where(x => DateTime.UtcNow - x.TransactionDate < TimeSpan.FromMinutes(1)).ToHashSet(x => x.Id); + + // remove recently rejected orders, sometimes we'll get updates for these but we've already marked them as rejected + stillUnknownOrderIDs.RemoveAll(x => recentOrders.Contains(x)); + + // the remaining ones were placed outside of the algorithm: those the algorithm was + // offered and didn't accept, and those we couldn't offer it at all + var unknownOrders = intradayOrders.ToDictionary(x => x.Id); var notAcceptedOrderIDs = new HashSet(); var unprocessableOrderIDs = new HashSet(); foreach (var stillUnknownOrderID in stillUnknownOrderIDs) { - if (!intradayOrders.TryGetValue(stillUnknownOrderID, out var unknownOrder)) + if (!unknownOrders.TryGetValue(stillUnknownOrderID, out var unknownOrder)) { // we don't have the details of the order, so we can't offer it to the algorithm unprocessableOrderIDs.Add(stillUnknownOrderID); continue; } - // skip recently rejected orders, sometimes we'll get updates for these but we've - // already marked them as rejected. We pull the orders every couple of seconds, so - // an older rejection is not one of ours, it was placed outside of the algorithm - if (unknownOrder.Status == TradierOrderStatus.Rejected - && DateTime.UtcNow - unknownOrder.TransactionDate < TimeSpan.FromMinutes(1)) - { - continue; - } - - // the order was placed outside of the algorithm, let it decide whether to take ownership of it + // let the algorithm decide whether it wants to take ownership of the order switch (HandleBrokerageSideOrder(unknownOrder)) { case BrokerageSideOrderResult.NotAccepted: From 38f1165b6cde3430213ecea299b24a8203cdd788 Mon Sep 17 00:00:00 2001 From: Alexandre Catarino Date: Fri, 7 Aug 2026 23:28:13 +0100 Subject: [PATCH 07/13] Address review feedback on the brokerage side order support - Move the BrokerageSideOrderResult enum to its own file under Models/Enums - Clone the brokerage order and reset the execution fields instead of re-creating it field by field - Report the order as submitted with the message the other brokerages use - Cache the order only after it holds its real state: it was being cached with no execution progress before its fills were emitted, so a concurrent poll could see an already filled order as unfilled and emit them twice - Cover the unsupported multileg only order types with a test - Only trace the ids pending a recheck when they change, every poll retries them so an unreachable orders endpoint would flood the log - Add a test for the ids being left unverified after a failed verification, which is what lets the next poll fire a new task for them Co-Authored-By: Claude Opus 5 (1M context) --- .../TradierBrokerageAditionalTests.cs | 90 ++++++++++++++++--- .../Models/Enums/BrokerageSideOrderResult.cs | 39 ++++++++ QuantConnect.TradierBrokerage/Orders.cs | 9 ++ .../TradierBrokerage.cs | 86 +++++++----------- 4 files changed, 160 insertions(+), 64 deletions(-) create mode 100644 QuantConnect.TradierBrokerage/Models/Enums/BrokerageSideOrderResult.cs diff --git a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs index 5b638de..2b73d80 100644 --- a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs +++ b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs @@ -14,6 +14,7 @@ */ using Moq; +using Newtonsoft.Json; using Newtonsoft.Json.Linq; using NUnit.Framework; using QuantConnect.Brokerages; @@ -364,7 +365,8 @@ public void NotifiesOrdersPlacedOutsideOfTheAlgorithm() orderProvider.Add(e.Order); }; - Assert.AreEqual("Tracked", InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m))); + Assert.AreEqual(BrokerageSideOrderResult.Tracked, + InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m))); Assert.IsNotNull(notifiedOrder); Assert.AreEqual(OrderType.Market, notifiedOrder.Type); @@ -390,7 +392,8 @@ public void TracksOpenOrdersPlacedOutsideOfTheAlgorithmForFutureFills() brokerage.OrdersStatusChanged += (_, events) => orderEvents.AddRange(events); brokerage.NewBrokerageOrderNotification += (_, e) => orderProvider.Add(e.Order); - Assert.AreEqual("Tracked", InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Open))); + Assert.AreEqual(BrokerageSideOrderResult.Tracked, + InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Open))); // the order is still open, so it's only reported as submitted, no fill event yet Assert.AreEqual(1, orderEvents.Count); @@ -414,7 +417,8 @@ public void DoesNotTrackOrdersPlacedOutsideOfTheAlgorithmWhenTheyAreNotAccepted( // the default brokerage message handler ignores these orders, leaving the Lean order id unset brokerage.NewBrokerageOrderNotification += (_, e) => notified = true; - Assert.AreEqual("NotAccepted", InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m))); + Assert.AreEqual(BrokerageSideOrderResult.NotAccepted, + InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m))); Assert.IsTrue(notified); Assert.IsEmpty(orderEvents); @@ -423,8 +427,10 @@ public void DoesNotTrackOrdersPlacedOutsideOfTheAlgorithmWhenTheyAreNotAccepted( // Orders LEAN cannot represent, like the multileg only order types, are never offered to the algorithm, so they // are reported apart from the ones it declined: telling the user to accept them in a handler wouldn't help - [Test] - public void ReportsOrdersItCannotConvertAsUnprocessable() + [TestCase(TradierOrderType.Credit)] + [TestCase(TradierOrderType.Debit)] + [TestCase(TradierOrderType.Even)] + public void ReportsUnsupportedOrderTypesAsUnprocessable(TradierOrderType unsupportedType) { var brokerage = CreateBrokerageWithOrderTracking(new OrderProvider()); @@ -434,14 +440,66 @@ public void ReportsOrdersItCannotConvertAsUnprocessable() var notified = false; brokerage.NewBrokerageOrderNotification += (_, e) => notified = true; - var brokerageSideOrder = CreateBrokerageSideOrder(TradierOrderStatus.Open, type: TradierOrderType.Credit); - Assert.AreEqual("Unprocessable", InvokeHandleBrokerageSideOrder(brokerage, brokerageSideOrder)); + var brokerageSideOrder = CreateBrokerageSideOrder(TradierOrderStatus.Open, type: unsupportedType); + Assert.AreEqual(BrokerageSideOrderResult.Unprocessable, InvokeHandleBrokerageSideOrder(brokerage, brokerageSideOrder)); Assert.IsFalse(notified); Assert.IsEmpty(orderEvents); Assert.IsFalse(GetCachedOpenOrders(brokerage).Contains(BrokerageSideOrderId)); } + // A failed verification used to add the unknown ids back to the pending set, but a new verification task only + // fires when that set is empty, so a single failure disabled the unknown order detection for the rest of the run + [Test] + public void RechecksUnknownOrderIdsAfterAFailedVerification() + { + var brokerageSideOrder = CreateBrokerageSideOrder(TradierOrderStatus.Open); + // the order has to look newer than the brokerage instance for the fill polling to flag it + brokerageSideOrder.TransactionDate = DateTime.UtcNow.AddHours(1); + + var restClient = new Mock(); + restClient.Setup(x => x.Execute(It.IsAny())).Returns(() => CreateResponse(SerializeOrders(brokerageSideOrder))); + + var messages = new List(); + var verificationFailed = new ManualResetEvent(false); + var brokerage = CreateBrokerageWithRestClient(restClient.Object, new List()); + brokerage.Message += (_, e) => + { + lock (messages) { messages.Add(e); } + if (e.Code == "UnknownIdResolution") + { + verificationFailed.Set(); + } + }; + + // fail the verification: the order provider lookup is the first thing the task does + var orderProvider = new Mock(); + orderProvider.Setup(x => x.GetOrdersByBrokerageId(It.IsAny())).Throws(new Exception("Verification failed")); + SetPrivateField(typeof(TradierBrokerage), brokerage, "_orderProvider", orderProvider.Object); + + var cachedOpenOrders = typeof(TradierBrokerage).GetField("_cachedOpenOrdersByTradierOrderID", + BindingFlags.NonPublic | BindingFlags.Instance); + cachedOpenOrders.SetValue(brokerage, Activator.CreateInstance(cachedOpenOrders.FieldType)); + + InvokeCheckForFills(brokerage); + + // the verification runs two seconds after the order is flagged + Assert.IsTrue(verificationFailed.WaitOne(TimeSpan.FromSeconds(30)), + "The verification was never attempted. Messages: " + string.Join(" | ", messages.Select(x => $"{x.Type}:{x.Code}:{x.Message}"))); + + // leaving the id out of the pending set is what lets the next poll fire a new verification task for it + var pendingOrderIDs = (HashSet)typeof(TradierBrokerage) + .GetField("_unknownTradierOrderIDs", BindingFlags.NonPublic | BindingFlags.Instance) + .GetValue(brokerage); + Assert.IsEmpty(pendingOrderIDs); + } + + // the orders container is only meant to be deserialized, its converter doesn't write, so wrap the orders by hand + private static string SerializeOrders(params TradierOrder[] orders) + { + return $"{{\"orders\":{{\"order\":{JsonConvert.SerializeObject(orders)}}}}}"; + } + private static TradierOrder CreateBrokerageSideOrder(TradierOrderStatus status, decimal quantityExecuted = 0m, TradierOrderType type = TradierOrderType.Market) { @@ -489,13 +547,12 @@ private static IDictionary GetCachedOpenOrders(TradierBrokerage brokerage) .GetValue(brokerage); } - // the result is a private enum, so it's compared by name - private static string InvokeHandleBrokerageSideOrder(TradierBrokerage brokerage, TradierOrder brokerageSideOrder) + private static BrokerageSideOrderResult InvokeHandleBrokerageSideOrder(TradierBrokerage brokerage, TradierOrder brokerageSideOrder) { var method = typeof(TradierBrokerage).GetMethod("HandleBrokerageSideOrder", BindingFlags.NonPublic | BindingFlags.Instance); try { - return method.Invoke(brokerage, [brokerageSideOrder]).ToString(); + return (BrokerageSideOrderResult)method.Invoke(brokerage, [brokerageSideOrder]); } catch (TargetInvocationException e) { @@ -583,6 +640,19 @@ private static ManualResetEventSlim SignalFirstResponse(TradierBrokerage brokera } } + private static void InvokeCheckForFills(TradierBrokerage brokerage) + { + var method = typeof(TradierBrokerage).GetMethod("CheckForFills", BindingFlags.NonPublic | BindingFlags.Instance); + try + { + method.Invoke(brokerage, null); + } + catch (TargetInvocationException e) + { + throw e.InnerException; + } + } + private static void SetPrivateField(Type type, object instance, string name, object value) { type.GetField(name, BindingFlags.NonPublic | BindingFlags.Instance).SetValue(instance, value); diff --git a/QuantConnect.TradierBrokerage/Models/Enums/BrokerageSideOrderResult.cs b/QuantConnect.TradierBrokerage/Models/Enums/BrokerageSideOrderResult.cs new file mode 100644 index 0000000..167e9e2 --- /dev/null +++ b/QuantConnect.TradierBrokerage/Models/Enums/BrokerageSideOrderResult.cs @@ -0,0 +1,39 @@ +/* + * QUANTCONNECT.COM - Democratizing Finance, Empowering Individuals. + * Lean Algorithmic Trading Engine v2.0. Copyright 2014 QuantConnect Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * +*/ + +namespace QuantConnect.Brokerages.Tradier +{ + /// + /// The outcome of offering an order placed outside of the algorithm to it + /// + public enum BrokerageSideOrderResult + { + /// + /// The algorithm took ownership of the order and we started tracking it + /// + Tracked, + + /// + /// The algorithm was offered the order and didn't accept it + /// + NotAccepted, + + /// + /// The order couldn't be offered to the algorithm, we failed to convert it into a Lean order + /// + Unprocessable + } +} diff --git a/QuantConnect.TradierBrokerage/Orders.cs b/QuantConnect.TradierBrokerage/Orders.cs index c7b097b..a3a38db 100644 --- a/QuantConnect.TradierBrokerage/Orders.cs +++ b/QuantConnect.TradierBrokerage/Orders.cs @@ -139,6 +139,15 @@ public class TradierOrder /// Closed position trade summary public TradierOrder() { } + + /// + /// Creates a shallow copy of this order + /// + /// A new instance holding the same values + public TradierOrder Clone() + { + return (TradierOrder)MemberwiseClone(); + } } /// diff --git a/QuantConnect.TradierBrokerage/TradierBrokerage.cs b/QuantConnect.TradierBrokerage/TradierBrokerage.cs index 9584014..0d7c8d6 100644 --- a/QuantConnect.TradierBrokerage/TradierBrokerage.cs +++ b/QuantConnect.TradierBrokerage/TradierBrokerage.cs @@ -112,6 +112,9 @@ public partial class TradierBrokerage : BaseWebsocketsBrokerage, IDataQueueHandl private readonly HashSet _unknownTradierOrderIDs = new HashSet(); private readonly FixedSizeHashQueue _verifiedUnknownTradierOrderIDs = new FixedSizeHashQueue(1000); + + // the ids we last reported as pending a recheck, we only trace them again when they change + private string _uncheckedTradierOrderIDs; private readonly FixedSizeHashQueue _cancelledQcOrderIDs = new FixedSizeHashQueue(10000); private string _restApiUrl = "https://api.tradier.com/v1/"; private string _restApiSandboxUrl = "https://sandbox.tradier.com/v1/"; @@ -1279,11 +1282,17 @@ private void CheckForFills() var intradayOrders = GetIntradayAndPendingOrders(); if (intradayOrders.Count == 0) { - // the request failed and already reported it, leave these ids unverified so we recheck them - Log.Trace("TradierBrokerage.CheckForFills(): No orders returned, will recheck the missing brokerage IDs: " - + string.Join(", ", stillUnknownOrderIDs)); + // the request failed and already reported it, leave these ids unverified so we recheck them. + // every poll rechecks them, so only trace when the ids change, else we would flood the log + var uncheckedOrderIDs = string.Join(", ", stillUnknownOrderIDs); + if (_uncheckedTradierOrderIDs != uncheckedOrderIDs) + { + _uncheckedTradierOrderIDs = uncheckedOrderIDs; + Log.Trace("TradierBrokerage.CheckForFills(): No orders returned, will recheck the missing brokerage IDs: " + uncheckedOrderIDs); + } return; } + _uncheckedTradierOrderIDs = null; // fetch all rejected intraday orders within the last minute, we're going to exclude rejected orders from the error // condition. We pull the orders every couple of seconds, so an older rejection can't be one of ours @@ -1415,43 +1424,33 @@ private BrokerageSideOrderResult HandleBrokerageSideOrder(TradierOrder brokerage return BrokerageSideOrderResult.NotAccepted; } - OnOrderEvent(new OrderEvent(leanOrder, DateTime.UtcNow, OrderFee.Zero, "Order was submitted outside of the algorithm") + OnOrderEvent(new OrderEvent(leanOrder, DateTime.UtcNow, OrderFee.Zero, "Order was submitted outside Lean") { Status = OrderStatus.Submitted }); - // start tracking the order from a state with no execution progress, this way the regular fill detection - // emits the fills and the final status the order might already have - var cachedOrder = new TradierCachedOpenOrder(new TradierOrder - { - Id = brokerageSideOrder.Id, - Type = brokerageSideOrder.Type, - Symbol = brokerageSideOrder.Symbol, - OptionSymbol = brokerageSideOrder.OptionSymbol, - Direction = brokerageSideOrder.Direction, - Quantity = brokerageSideOrder.Quantity, - // we just reported the order as submitted, keeping the status of orders that are still open avoids - // emitting an event for a status change that never happened - Status = OrderIsOpen(brokerageSideOrder) ? brokerageSideOrder.Status : TradierOrderStatus.Submitted, - Duration = brokerageSideOrder.Duration, - Price = brokerageSideOrder.Price, - AverageFillPrice = 0m, - QuantityExecuted = 0m, - LastFillPrice = 0m, - LastFillQuantity = 0m, - RemainingQuantity = brokerageSideOrder.Quantity, - CreatedDate = brokerageSideOrder.CreatedDate, - TransactionDate = brokerageSideOrder.TransactionDate, - Class = brokerageSideOrder.Class, - NumberOfLegs = brokerageSideOrder.NumberOfLegs, - Legs = brokerageSideOrder.Legs - }); - _cachedOpenOrdersByTradierOrderID[brokerageSideOrder.Id] = cachedOrder; + // diff the order against a clone of itself with no execution progress, this way the regular fill + // detection emits the fills and the final status the order might already have + var brokerageSideOrderClone = brokerageSideOrder.Clone(); + brokerageSideOrderClone.AverageFillPrice = 0m; + brokerageSideOrderClone.QuantityExecuted = 0m; + brokerageSideOrderClone.LastFillPrice = 0m; + brokerageSideOrderClone.LastFillQuantity = 0m; + brokerageSideOrderClone.RemainingQuantity = brokerageSideOrder.Quantity; + if (OrderIsClosed(brokerageSideOrder)) + { + // we just reported the order as submitted, keeping the status of the orders that are still + // open avoids emitting an event for a status change that never happened + brokerageSideOrderClone.Status = TradierOrderStatus.Submitted; + } + var cachedOrder = new TradierCachedOpenOrder(brokerageSideOrderClone); ProcessPotentiallyUpdatedOrder(cachedOrder, brokerageSideOrder); - // if the order is still open, update the cached value + // only start tracking the order once it holds its real state, caching it any earlier would expose + // the clone to the fill polling, which would emit the fills we just emitted again if (!OrderIsClosed(brokerageSideOrder)) { - UpdateCachedOpenOrder(brokerageSideOrder.Id, brokerageSideOrder); + cachedOrder.Order = brokerageSideOrder; + _cachedOpenOrdersByTradierOrderID[brokerageSideOrder.Id] = cachedOrder; } return BrokerageSideOrderResult.Tracked; @@ -2086,27 +2085,6 @@ public TradierPlaceOrderRequest Next() } } - /// - /// The outcome of offering an order placed outside of the algorithm to it - /// - private enum BrokerageSideOrderResult - { - /// - /// The algorithm took ownership of the order and we started tracking it - /// - Tracked, - - /// - /// The algorithm was offered the order and didn't accept it - /// - NotAccepted, - - /// - /// The order couldn't be offered to the algorithm, we failed to convert it into a Lean order - /// - Unprocessable - } - private class TradierCachedOpenOrder { public bool EmittedOrderFee; From a465acb2f9114a6f8791d3e9dbf9b71d1ede0891 Mon Sep 17 00:00:00 2001 From: Alexandre Catarino Date: Fri, 11 Sep 2026 18:49:33 +0100 Subject: [PATCH 08/13] Address review: warn on declined brokerage-side orders, fix verification races - Declined orders are a one-time Warning and their ids are verified, so later polls don't re-offer them; unprocessable ones stay an Error - Clear the pending id set once the verification task ends, so a second task can't offer the same order twice; handle orders under the fill lock - Reject multileg/combo classes in ConvertOrder; GetOpenOrders skips orders it can't convert instead of failing the launch - Emit the partial fill of closed brokerage-side orders before the final status; cache the order before emitting so a failure is recoverable - TryGetIntradayAndPendingOrders tells a failed request from no orders - Shorter messages, private result enum, no Clone on the order model Co-Authored-By: Claude Fable 5.1 --- .../TradierBrokerageAditionalTests.cs | 186 +++++++++---- .../Models/Enums/BrokerageSideOrderResult.cs | 39 --- QuantConnect.TradierBrokerage/Orders.cs | 9 - .../TradierBrokerage.cs | 252 +++++++++++------- 4 files changed, 285 insertions(+), 201 deletions(-) delete mode 100644 QuantConnect.TradierBrokerage/Models/Enums/BrokerageSideOrderResult.cs diff --git a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs index 2b73d80..7a00d72 100644 --- a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs +++ b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs @@ -42,7 +42,6 @@ namespace QuantConnect.Tests.Brokerages.Tradier public class TradierBrokerageAditionalTests { private const long BrokerageSideOrderId = 1234; - private static readonly string[] BrokerageSideOrderBrokerIds = [BrokerageSideOrderId.ToStringInvariant()]; [Test] public void InitializesFactoryFromComposer() @@ -345,16 +344,15 @@ public void ReplacesProxyHtmlPageWithActionableMessageAfterRetriesAreExhausted(s restClient.Verify(x => x.Execute(It.IsAny()), Times.Exactly(2)); } - // Orders placed directly in the Tradier account, outside of the algorithm, are offered to the algorithm through - // the NewBrokerageOrderNotification event, so a brokerage message handler can take ownership of them instead of - // the algorithm being terminated for interference it doesn't control + // Orders placed in the Tradier account outside of the algorithm are offered to it through the + // NewBrokerageOrderNotification event, so a brokerage message handler can take ownership of them [Test] public void NotifiesOrdersPlacedOutsideOfTheAlgorithm() { var orderProvider = new OrderProvider(); var brokerage = CreateBrokerageWithOrderTracking(orderProvider); - var orderEvents = new List(); + List orderEvents = []; brokerage.OrdersStatusChanged += (_, events) => orderEvents.AddRange(events); Order notifiedOrder = null; @@ -365,14 +363,14 @@ public void NotifiesOrdersPlacedOutsideOfTheAlgorithm() orderProvider.Add(e.Order); }; - Assert.AreEqual(BrokerageSideOrderResult.Tracked, + Assert.AreEqual("Tracked", InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m))); Assert.IsNotNull(notifiedOrder); Assert.AreEqual(OrderType.Market, notifiedOrder.Type); Assert.AreEqual("SPY", notifiedOrder.Symbol.Value); Assert.AreEqual(10m, notifiedOrder.Quantity); - CollectionAssert.AreEqual(BrokerageSideOrderBrokerIds, notifiedOrder.BrokerId); + Assert.AreEqual(BrokerageSideOrderId.ToStringInvariant(), notifiedOrder.BrokerId.Single()); // the order is first reported as submitted and then the fill it already had when we found it is emitted Assert.AreEqual(2, orderEvents.Count); @@ -380,6 +378,7 @@ public void NotifiesOrdersPlacedOutsideOfTheAlgorithm() Assert.AreEqual(OrderStatus.Filled, orderEvents[1].Status); Assert.AreEqual(10m, orderEvents[1].FillQuantity); Assert.AreEqual(123.45m, orderEvents[1].FillPrice); + Assert.IsFalse(GetCachedOpenOrders(brokerage).Contains(BrokerageSideOrderId)); } [Test] @@ -388,12 +387,11 @@ public void TracksOpenOrdersPlacedOutsideOfTheAlgorithmForFutureFills() var orderProvider = new OrderProvider(); var brokerage = CreateBrokerageWithOrderTracking(orderProvider); - var orderEvents = new List(); + List orderEvents = []; brokerage.OrdersStatusChanged += (_, events) => orderEvents.AddRange(events); brokerage.NewBrokerageOrderNotification += (_, e) => orderProvider.Add(e.Order); - Assert.AreEqual(BrokerageSideOrderResult.Tracked, - InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Open))); + Assert.AreEqual("Tracked", InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Open))); // the order is still open, so it's only reported as submitted, no fill event yet Assert.AreEqual(1, orderEvents.Count); @@ -403,21 +401,46 @@ public void TracksOpenOrdersPlacedOutsideOfTheAlgorithmForFutureFills() Assert.IsTrue(GetCachedOpenOrders(brokerage).Contains(BrokerageSideOrderId)); } - // When the order is not accepted, which is what the default brokerage message handler does, the order is left - // untracked and reported back as not accepted so the caller can fail the algorithm like it did before + // Lean only applies the fills of fill events, so the executed part of an order that closed without + // filling has to be reported on its own before the final status + [TestCase(TradierOrderStatus.Canceled, OrderStatus.Canceled)] + [TestCase(TradierOrderStatus.Expired, OrderStatus.Invalid)] + public void ReportsThePartialFillOfClosedOrdersPlacedOutsideOfTheAlgorithm(TradierOrderStatus status, OrderStatus expectedFinalStatus) + { + var orderProvider = new OrderProvider(); + var brokerage = CreateBrokerageWithOrderTracking(orderProvider); + + List orderEvents = []; + brokerage.OrdersStatusChanged += (_, events) => orderEvents.AddRange(events); + brokerage.NewBrokerageOrderNotification += (_, e) => orderProvider.Add(e.Order); + + Assert.AreEqual("Tracked", + InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(status, quantityExecuted: 4m))); + + Assert.AreEqual(3, orderEvents.Count); + Assert.AreEqual(OrderStatus.Submitted, orderEvents[0].Status); + Assert.AreEqual(OrderStatus.PartiallyFilled, orderEvents[1].Status); + Assert.AreEqual(4m, orderEvents[1].FillQuantity); + Assert.AreEqual(123.45m, orderEvents[1].FillPrice); + Assert.AreEqual(expectedFinalStatus, orderEvents[2].Status); + Assert.AreEqual(0m, orderEvents[2].FillQuantity); + Assert.IsFalse(GetCachedOpenOrders(brokerage).Contains(BrokerageSideOrderId)); + } + + // Not accepting the order is what the default brokerage message handler does, it's left untracked [Test] public void DoesNotTrackOrdersPlacedOutsideOfTheAlgorithmWhenTheyAreNotAccepted() { var brokerage = CreateBrokerageWithOrderTracking(new OrderProvider()); - var orderEvents = new List(); + List orderEvents = []; brokerage.OrdersStatusChanged += (_, events) => orderEvents.AddRange(events); var notified = false; // the default brokerage message handler ignores these orders, leaving the Lean order id unset brokerage.NewBrokerageOrderNotification += (_, e) => notified = true; - Assert.AreEqual(BrokerageSideOrderResult.NotAccepted, + Assert.AreEqual("NotAccepted", InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m))); Assert.IsTrue(notified); @@ -425,29 +448,78 @@ public void DoesNotTrackOrdersPlacedOutsideOfTheAlgorithmWhenTheyAreNotAccepted( Assert.IsFalse(GetCachedOpenOrders(brokerage).Contains(BrokerageSideOrderId)); } - // Orders LEAN cannot represent, like the multileg only order types, are never offered to the algorithm, so they - // are reported apart from the ones it declined: telling the user to accept them in a handler wouldn't help - [TestCase(TradierOrderType.Credit)] - [TestCase(TradierOrderType.Debit)] - [TestCase(TradierOrderType.Even)] - public void ReportsUnsupportedOrderTypesAsUnprocessable(TradierOrderType unsupportedType) + // Orders LEAN cannot represent are never offered to the algorithm: the multileg only order types, and the + // multileg classes, which Tradier reports on the underlying and would otherwise convert to a single order on it + [TestCase(TradierOrderType.Credit, TradierOrderClass.Multileg)] + [TestCase(TradierOrderType.Debit, TradierOrderClass.Multileg)] + [TestCase(TradierOrderType.Even, TradierOrderClass.Multileg)] + [TestCase(TradierOrderType.Market, TradierOrderClass.Multileg)] + [TestCase(TradierOrderType.Limit, TradierOrderClass.Combo)] + public void ReportsUnsupportedOrdersAsUnprocessable(TradierOrderType type, TradierOrderClass orderClass) { var brokerage = CreateBrokerageWithOrderTracking(new OrderProvider()); - var orderEvents = new List(); + List orderEvents = []; brokerage.OrdersStatusChanged += (_, events) => orderEvents.AddRange(events); var notified = false; brokerage.NewBrokerageOrderNotification += (_, e) => notified = true; - var brokerageSideOrder = CreateBrokerageSideOrder(TradierOrderStatus.Open, type: unsupportedType); - Assert.AreEqual(BrokerageSideOrderResult.Unprocessable, InvokeHandleBrokerageSideOrder(brokerage, brokerageSideOrder)); + var brokerageSideOrder = CreateBrokerageSideOrder(TradierOrderStatus.Open, type: type, orderClass: orderClass); + Assert.AreEqual("Unprocessable", InvokeHandleBrokerageSideOrder(brokerage, brokerageSideOrder)); Assert.IsFalse(notified); Assert.IsEmpty(orderEvents); Assert.IsFalse(GetCachedOpenOrders(brokerage).Contains(BrokerageSideOrderId)); } + // Declining the order is a valid outcome, so the algorithm keeps running: it's warned once and the id is + // marked as verified, else every poll would offer the order to the algorithm again + [Test] + public void WarnsOnceAboutOrdersPlacedOutsideOfTheAlgorithmThatItDoesNotAccept() + { + var brokerageSideOrder = CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m); + // the order has to look newer than the brokerage instance for the fill polling to flag it + brokerageSideOrder.TransactionDate = DateTime.UtcNow.AddHours(1); + + var restClient = new Mock(); + restClient.Setup(x => x.Execute(It.IsAny())).Returns(() => CreateResponse(SerializeOrders(brokerageSideOrder))); + + var brokerage = CreateBrokerageWithOrderTracking(new OrderProvider(), restClient.Object); + + var notifications = 0; + brokerage.NewBrokerageOrderNotification += (_, e) => Interlocked.Increment(ref notifications); + + List messages = []; + var warned = new ManualResetEvent(false); + brokerage.Message += (_, e) => + { + lock (messages) { messages.Add(e); } + if (e.Code == "UnknownOrderId") + { + warned.Set(); + } + }; + + InvokeCheckForFills(brokerage); + + Assert.IsTrue(warned.WaitOne(TimeSpan.FromSeconds(30)), + "The order was never reported. Messages: " + string.Join(" | ", messages.Select(x => $"{x.Type}:{x.Code}:{x.Message}"))); + var warning = messages.Single(x => x.Code == "UnknownOrderId"); + Assert.AreEqual(BrokerageMessageType.Warning, warning.Type); + Assert.IsTrue(warning.Message.Contains(BrokerageSideOrderId.ToStringInvariant()), warning.Message); + Assert.AreEqual(1, notifications); + + // the id is verified right after the warning + var verifiedOrderIDs = GetPrivateField>(typeof(TradierBrokerage), brokerage, "_verifiedUnknownTradierOrderIDs"); + Assert.IsTrue(SpinWait.SpinUntil(() => verifiedOrderIDs.Contains(BrokerageSideOrderId), TimeSpan.FromSeconds(5))); + + // so the next poll neither flags it nor offers it again + InvokeCheckForFills(brokerage); + Assert.IsEmpty(GetPrivateField>(typeof(TradierBrokerage), brokerage, "_unknownTradierOrderIDs")); + Assert.AreEqual(1, notifications); + } + // A failed verification used to add the unknown ids back to the pending set, but a new verification task only // fires when that set is empty, so a single failure disabled the unknown order detection for the rest of the run [Test] @@ -460,38 +532,38 @@ public void RechecksUnknownOrderIdsAfterAFailedVerification() var restClient = new Mock(); restClient.Setup(x => x.Execute(It.IsAny())).Returns(() => CreateResponse(SerializeOrders(brokerageSideOrder))); - var messages = new List(); - var verificationFailed = new ManualResetEvent(false); - var brokerage = CreateBrokerageWithRestClient(restClient.Object, new List()); + // fail the verification: the order provider lookup is the first thing the task does + var orderProvider = new Mock(); + orderProvider.Setup(x => x.GetOrdersByBrokerageId(It.IsAny())).Throws(new Exception("Verification failed")); + var brokerage = CreateBrokerageWithOrderTracking(orderProvider.Object, restClient.Object); + + List messages = []; + var verificationFailures = 0; + var verificationFailed = new AutoResetEvent(false); brokerage.Message += (_, e) => { lock (messages) { messages.Add(e); } if (e.Code == "UnknownIdResolution") { + Interlocked.Increment(ref verificationFailures); verificationFailed.Set(); } }; - // fail the verification: the order provider lookup is the first thing the task does - var orderProvider = new Mock(); - orderProvider.Setup(x => x.GetOrdersByBrokerageId(It.IsAny())).Throws(new Exception("Verification failed")); - SetPrivateField(typeof(TradierBrokerage), brokerage, "_orderProvider", orderProvider.Object); - - var cachedOpenOrders = typeof(TradierBrokerage).GetField("_cachedOpenOrdersByTradierOrderID", - BindingFlags.NonPublic | BindingFlags.Instance); - cachedOpenOrders.SetValue(brokerage, Activator.CreateInstance(cachedOpenOrders.FieldType)); - InvokeCheckForFills(brokerage); // the verification runs two seconds after the order is flagged Assert.IsTrue(verificationFailed.WaitOne(TimeSpan.FromSeconds(30)), "The verification was never attempted. Messages: " + string.Join(" | ", messages.Select(x => $"{x.Type}:{x.Code}:{x.Message}"))); - // leaving the id out of the pending set is what lets the next poll fire a new verification task for it - var pendingOrderIDs = (HashSet)typeof(TradierBrokerage) - .GetField("_unknownTradierOrderIDs", BindingFlags.NonPublic | BindingFlags.Instance) - .GetValue(brokerage); - Assert.IsEmpty(pendingOrderIDs); + // the failed task releases the ids once it's done, keep polling like the fill timer does until a new one fires + var deadline = DateTime.UtcNow.AddSeconds(30); + while (!verificationFailed.WaitOne(TimeSpan.FromMilliseconds(100))) + { + Assert.Less(DateTime.UtcNow, deadline, "The verification was never retried"); + InvokeCheckForFills(brokerage); + } + Assert.AreEqual(2, verificationFailures); } // the orders container is only meant to be deserialized, its converter doesn't write, so wrap the orders by hand @@ -501,7 +573,7 @@ private static string SerializeOrders(params TradierOrder[] orders) } private static TradierOrder CreateBrokerageSideOrder(TradierOrderStatus status, decimal quantityExecuted = 0m, - TradierOrderType type = TradierOrderType.Market) + TradierOrderType type = TradierOrderType.Market, TradierOrderClass orderClass = TradierOrderClass.Equity) { return new TradierOrder { @@ -516,7 +588,7 @@ private static TradierOrder CreateBrokerageSideOrder(TradierOrderStatus status, RemainingQuantity = 10m - quantityExecuted, LastFillPrice = quantityExecuted > 0 ? 123.45m : 0m, AverageFillPrice = quantityExecuted > 0 ? 123.45m : 0m, - Class = TradierOrderClass.Equity, + Class = orderClass, TransactionDate = DateTime.UtcNow, CreatedDate = DateTime.UtcNow }; @@ -524,9 +596,9 @@ private static TradierOrder CreateBrokerageSideOrder(TradierOrderStatus status, // Builds a brokerage with just the state the brokerage side order handling needs, skipping the heavy Initialize // (license validation, timers, streaming threads) that a full construction would trigger. - private static TradierBrokerage CreateBrokerageWithOrderTracking(OrderProvider orderProvider) + private static TradierBrokerage CreateBrokerageWithOrderTracking(IOrderProvider orderProvider, IRestClient restClient = null) { - var brokerage = new TradierBrokerage(); + var brokerage = restClient == null ? new TradierBrokerage() : CreateBrokerageWithRestClient(restClient, []); SetPrivateField(typeof(TradierBrokerage), brokerage, "_orderProvider", orderProvider); SetPrivateField(typeof(TradierBrokerage), brokerage, "_securityProvider", new SecurityProvider()); @@ -542,22 +614,13 @@ private static TradierBrokerage CreateBrokerageWithOrderTracking(OrderProvider o private static IDictionary GetCachedOpenOrders(TradierBrokerage brokerage) { - return (IDictionary)typeof(TradierBrokerage) - .GetField("_cachedOpenOrdersByTradierOrderID", BindingFlags.NonPublic | BindingFlags.Instance) - .GetValue(brokerage); + return GetPrivateField(typeof(TradierBrokerage), brokerage, "_cachedOpenOrdersByTradierOrderID"); } - private static BrokerageSideOrderResult InvokeHandleBrokerageSideOrder(TradierBrokerage brokerage, TradierOrder brokerageSideOrder) + // the result type is private to the brokerage, so compare its name + private static string InvokeHandleBrokerageSideOrder(TradierBrokerage brokerage, TradierOrder brokerageSideOrder) { - var method = typeof(TradierBrokerage).GetMethod("HandleBrokerageSideOrder", BindingFlags.NonPublic | BindingFlags.Instance); - try - { - return (BrokerageSideOrderResult)method.Invoke(brokerage, [brokerageSideOrder]); - } - catch (TargetInvocationException e) - { - throw e.InnerException; - } + return InvokePrivate(brokerage, "HandleBrokerageSideOrder", brokerageSideOrder).ToString(); } private static IRestResponse CreateResponse(string content, HttpStatusCode statusCode = HttpStatusCode.OK) @@ -642,10 +705,15 @@ private static ManualResetEventSlim SignalFirstResponse(TradierBrokerage brokera private static void InvokeCheckForFills(TradierBrokerage brokerage) { - var method = typeof(TradierBrokerage).GetMethod("CheckForFills", BindingFlags.NonPublic | BindingFlags.Instance); + InvokePrivate(brokerage, "CheckForFills"); + } + + private static object InvokePrivate(TradierBrokerage brokerage, string name, params object[] args) + { + var method = typeof(TradierBrokerage).GetMethod(name, BindingFlags.NonPublic | BindingFlags.Instance); try { - method.Invoke(brokerage, null); + return method.Invoke(brokerage, args); } catch (TargetInvocationException e) { diff --git a/QuantConnect.TradierBrokerage/Models/Enums/BrokerageSideOrderResult.cs b/QuantConnect.TradierBrokerage/Models/Enums/BrokerageSideOrderResult.cs deleted file mode 100644 index 167e9e2..0000000 --- a/QuantConnect.TradierBrokerage/Models/Enums/BrokerageSideOrderResult.cs +++ /dev/null @@ -1,39 +0,0 @@ -/* - * QUANTCONNECT.COM - Democratizing Finance, Empowering Individuals. - * Lean Algorithmic Trading Engine v2.0. Copyright 2014 QuantConnect Corporation. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * -*/ - -namespace QuantConnect.Brokerages.Tradier -{ - /// - /// The outcome of offering an order placed outside of the algorithm to it - /// - public enum BrokerageSideOrderResult - { - /// - /// The algorithm took ownership of the order and we started tracking it - /// - Tracked, - - /// - /// The algorithm was offered the order and didn't accept it - /// - NotAccepted, - - /// - /// The order couldn't be offered to the algorithm, we failed to convert it into a Lean order - /// - Unprocessable - } -} diff --git a/QuantConnect.TradierBrokerage/Orders.cs b/QuantConnect.TradierBrokerage/Orders.cs index a3a38db..c7b097b 100644 --- a/QuantConnect.TradierBrokerage/Orders.cs +++ b/QuantConnect.TradierBrokerage/Orders.cs @@ -139,15 +139,6 @@ public class TradierOrder /// Closed position trade summary public TradierOrder() { } - - /// - /// Creates a shallow copy of this order - /// - /// A new instance holding the same values - public TradierOrder Clone() - { - return (TradierOrder)MemberwiseClone(); - } } /// diff --git a/QuantConnect.TradierBrokerage/TradierBrokerage.cs b/QuantConnect.TradierBrokerage/TradierBrokerage.cs index 0d7c8d6..a94b401 100644 --- a/QuantConnect.TradierBrokerage/TradierBrokerage.cs +++ b/QuantConnect.TradierBrokerage/TradierBrokerage.cs @@ -112,9 +112,6 @@ public partial class TradierBrokerage : BaseWebsocketsBrokerage, IDataQueueHandl private readonly HashSet _unknownTradierOrderIDs = new HashSet(); private readonly FixedSizeHashQueue _verifiedUnknownTradierOrderIDs = new FixedSizeHashQueue(1000); - - // the ids we last reported as pending a recheck, we only trace them again when they change - private string _uncheckedTradierOrderIDs; private readonly FixedSizeHashQueue _cancelledQcOrderIDs = new FixedSizeHashQueue(10000); private string _restApiUrl = "https://api.tradier.com/v1/"; private string _restApiSandboxUrl = "https://sandbox.tradier.com/v1/"; @@ -423,19 +420,28 @@ public List GetGainLoss() /// /// Get Intraday and pending orders for users account: accounts/{account_id}/orders /// - private List GetIntradayAndPendingOrders() + /// The orders, empty if there are none or the request failed + /// False if the request failed, the failure was already reported + private bool TryGetIntradayAndPendingOrders(out List orders) { var request = new RestRequest($"accounts/{_accountId}/orders"); var ordersContainer = Execute(request, TradierApiRequestType.Standard); - if (ordersContainer?.Orders == null) + orders = []; + if (ordersContainer == null) + { + return false; + } + + if (ordersContainer.Orders == null) { // we had a successful call but there weren't any orders returned Log.Trace("Tradier.FetchOrders(): No orders found"); - return new List(); + return true; } - return ordersContainer.Orders.Orders; + orders = ordersContainer.Orders.Orders; + return true; } /// @@ -789,13 +795,28 @@ private bool TryDeserializeRemoveRoot(string json, string rootName, out T obj public override List GetOpenOrders() { var orders = new List(); - var openOrders = GetIntradayAndPendingOrders().Where(OrderIsOpen); + if (!TryGetIntradayAndPendingOrders(out var intradayAndPendingOrders)) + { + return orders; + } + + var openOrders = intradayAndPendingOrders.Where(OrderIsOpen); foreach (var openOrder in openOrders) { + try + { + orders.Add(ConvertOrder(openOrder)); + } + catch (Exception err) + { + // an order Lean can't represent shouldn't keep the algorithm from launching, the fill polling reports it + Log.Error(err, $"skipping Tradier order {openOrder.Id}"); + continue; + } + // make sure our internal collection is up to date as well UpdateCachedOpenOrder(openOrder.Id, openOrder); - orders.Add(ConvertOrder(openOrder)); } return orders; @@ -1158,7 +1179,11 @@ out var rawContent { Task.Run(() => { - var orders = GetIntradayAndPendingOrders() + if (!TryGetIntradayAndPendingOrders(out var intradayAndPendingOrders)) + { + return; + } + var orders = intradayAndPendingOrders .Where(x => x.Status == TradierOrderStatus.Rejected) .Where(x => DateTime.UtcNow - x.TransactionDate < TimeSpan.FromSeconds(2)); @@ -1192,10 +1217,8 @@ private void CheckForFills() try { - var intradayAndPendingOrders = GetIntradayAndPendingOrders(); - if (intradayAndPendingOrders == null) + if (!TryGetIntradayAndPendingOrders(out var intradayAndPendingOrders)) { - Log.Error("TradierBrokerage.CheckForFills(): Returned null response!"); return; } @@ -1256,10 +1279,12 @@ private void CheckForFills() // if we get order updates for orders we're unaware of we need to bail, this can corrupt the algorithm state var unknownOrderIDs = updatedOrders.Where(IsUnknownOrderID).ToHashSet(x => x.Key); unknownOrderIDs.ExceptWith(_verifiedUnknownTradierOrderIDs); - var fireTask = unknownOrderIDs.Count != 0 && _unknownTradierOrderIDs.Count == 0; - foreach (var unknownOrderID in unknownOrderIDs) + bool fireTask; + lock (_unknownTradierOrderIDs) { - _unknownTradierOrderIDs.Add(unknownOrderID); + // a pending id means a verification task is in flight, it releases them all once it's done + fireTask = unknownOrderIDs.Count != 0 && _unknownTradierOrderIDs.Count == 0; + _unknownTradierOrderIDs.UnionWith(unknownOrderIDs); } if (fireTask) @@ -1267,8 +1292,11 @@ private void CheckForFills() // wait a second and then check the order provider to see if we have these broker IDs, maybe they came in later (ex, symbol denied for short trading) Task.Delay(TimeSpan.FromSeconds(2)).ContinueWith(t => { - var localUnknownTradierOrderIDs = _unknownTradierOrderIDs.ToHashSet(); - _unknownTradierOrderIDs.Clear(); + HashSet localUnknownTradierOrderIDs; + lock (_unknownTradierOrderIDs) + { + localUnknownTradierOrderIDs = [.. _unknownTradierOrderIDs]; + } try { // verify we don't have them in the order provider @@ -1279,20 +1307,11 @@ private void CheckForFills() // so don't treat orders placed externally in the account as an error if (_orderProvider != null && stillUnknownOrderIDs.Count > 0) { - var intradayOrders = GetIntradayAndPendingOrders(); - if (intradayOrders.Count == 0) + if (!TryGetIntradayAndPendingOrders(out var intradayOrders)) { - // the request failed and already reported it, leave these ids unverified so we recheck them. - // every poll rechecks them, so only trace when the ids change, else we would flood the log - var uncheckedOrderIDs = string.Join(", ", stillUnknownOrderIDs); - if (_uncheckedTradierOrderIDs != uncheckedOrderIDs) - { - _uncheckedTradierOrderIDs = uncheckedOrderIDs; - Log.Trace("TradierBrokerage.CheckForFills(): No orders returned, will recheck the missing brokerage IDs: " + uncheckedOrderIDs); - } + // leave these ids unverified so the next poll rechecks them return; } - _uncheckedTradierOrderIDs = null; // fetch all rejected intraday orders within the last minute, we're going to exclude rejected orders from the error // condition. We pull the orders every couple of seconds, so an older rejection can't be one of ours @@ -1302,74 +1321,74 @@ private void CheckForFills() // remove recently rejected orders, sometimes we'll get updates for these but we've already marked them as rejected stillUnknownOrderIDs.RemoveAll(x => recentOrders.Contains(x)); - // the remaining ones were placed outside of the algorithm: those the algorithm was - // offered and didn't accept, and those we couldn't offer it at all + // the remaining ones were placed outside of the algorithm var unknownOrders = intradayOrders.ToDictionary(x => x.Id); - var notAcceptedOrderIDs = new HashSet(); - var unprocessableOrderIDs = new HashSet(); - foreach (var stillUnknownOrderID in stillUnknownOrderIDs) + HashSet notAcceptedOrderIDs = []; + HashSet unprocessableOrderIDs = []; + // the fill polling diffs the cached orders, hold its lock so it can't see one we're still setting up + lock (_fillLock) { - if (!unknownOrders.TryGetValue(stillUnknownOrderID, out var unknownOrder)) + foreach (var stillUnknownOrderID in stillUnknownOrderIDs) { - // we don't have the details of the order, so we can't offer it to the algorithm - unprocessableOrderIDs.Add(stillUnknownOrderID); - continue; - } - - // let the algorithm decide whether it wants to take ownership of the order - switch (HandleBrokerageSideOrder(unknownOrder)) - { - case BrokerageSideOrderResult.NotAccepted: - notAcceptedOrderIDs.Add(stillUnknownOrderID); - break; - - case BrokerageSideOrderResult.Unprocessable: + if (!unknownOrders.TryGetValue(stillUnknownOrderID, out var unknownOrder)) + { + // we don't have the details of the order, so we can't offer it to the algorithm unprocessableOrderIDs.Add(stillUnknownOrderID); - break; + continue; + } + + switch (HandleBrokerageSideOrder(unknownOrder)) + { + case BrokerageSideOrderResult.NotAccepted: + notAcceptedOrderIDs.Add(stillUnknownOrderID); + break; + + case BrokerageSideOrderResult.Unprocessable: + unprocessableOrderIDs.Add(stillUnknownOrderID); + break; + } } } + if (notAcceptedOrderIDs.Count > 0) + { + // declining an order placed outside of the algorithm is a valid outcome, we just won't track it + var ids = string.Join(", ", notAcceptedOrderIDs); + Log.Trace("TradierBrokerage.CheckForFills(): Orders placed outside of the algorithm were not accepted by it: " + ids); + OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Warning, "UnknownOrderId", + $"Tradier order id(s) {ids} were placed outside of the algorithm and were not accepted by its brokerage message handler, so they will not be tracked.")); + } + if (unprocessableOrderIDs.Count > 0) { // we couldn't hand these orders over to the algorithm, so we've gotta bail on it var ids = string.Join(", ", unprocessableOrderIDs); Log.Error("TradierBrokerage.CheckForFills(): Unable to process the missing brokerage IDs: " + ids); - OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Error, "UnprocessableOrderId", "Unable to process the Tradier order id(s): " + ids + - ". Their details could not be fetched from Tradier or they cannot be represented as LEAN orders, so the algorithm cannot track them." + - " LEAN terminates live algorithms when it detects interference outside of the algorithm's control to avoid race conditions between" + - " the account owner and the algorithm, so avoid placing manual orders while the algorithm is running.")); - return; - } - - if (notAcceptedOrderIDs.Count > 0) - { - // the algorithm didn't take ownership of these orders, so we've gotta bail on it - var ids = string.Join(", ", notAcceptedOrderIDs); - Log.Error("TradierBrokerage.CheckForFills(): Unable to verify all missing brokerage IDs: " + ids); - OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Error, "UnknownOrderId", "Received unknown Tradier order id(s): " + ids + - ". These orders were likely placed manually on the account. LEAN terminates live algorithms when it detects interference outside of the algorithm's control" + - " to avoid race conditions between the account owner and the algorithm, so avoid placing manual orders while the algorithm is running." + - " To adjust your holdings manually, place your trades through the QuantConnect IDE or LEAN CLI, or stop the algorithm, place your trades, and then redeploy." + - " If instead you want the algorithm to take ownership of the orders placed outside of it, set a custom brokerage message handler with" + - " 'SetBrokerageMessageHandler(...)' implementing a 'HandleOrder' method that returns true for the orders the algorithm should track.")); - return; + OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Error, "UnprocessableOrderId", + $"Tradier order id(s) {ids} were placed outside of the algorithm and cannot be represented as LEAN orders.")); } } + + // add these to the verified list so we don't check them again, the reported ones included: their outcome won't change foreach (var unknownTradierOrderID in localUnknownTradierOrderIDs) { - // add these to the verified list so we don't check them again _verifiedUnknownTradierOrderIDs.Add(unknownTradierOrderID); } Log.Trace("TradierBrokerage.CheckForFills(): Verified all missing brokerage IDs."); } catch (Exception err) { - // we need to recheck these order ids since we failed, leaving them unverified is enough for the next - // poll to fire a new task for them, adding them back to the set would block that task from firing - + // the ids stay unverified, so the next poll fires a new task for them Log.Error(err); OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Warning, "UnknownIdResolution", "An error occurred while trying to resolve unknown Tradier order IDs: " + err)); } + finally + { + lock (_unknownTradierOrderIDs) + { + _unknownTradierOrderIDs.Clear(); + } + } }); } } @@ -1410,52 +1429,91 @@ private BrokerageSideOrderResult HandleBrokerageSideOrder(TradierOrder brokerage } catch (Exception err) { - Log.Error(err, $"TradierBrokerage.HandleBrokerageSideOrder(): failed to convert Tradier order {brokerageSideOrder.Id}"); + Log.Error(err, $"failed to convert Tradier order {brokerageSideOrder.Id}"); + return BrokerageSideOrderResult.Unprocessable; + } + + // GetOrder returns a blank order when the details request fails, we'd offer a stop at zero + if (leanOrder is StopMarketOrder { StopPrice: 0 } or StopLimitOrder { StopPrice: 0 }) + { + Log.Error($"TradierBrokerage.HandleBrokerageSideOrder(): could not fetch the stop price of Tradier order {brokerageSideOrder.Id}"); return BrokerageSideOrderResult.Unprocessable; } - // the transaction handler assigns the Lean order id and marks the order as submitted when it accepts it + // the transaction handler only normalizes a New status to Submitted when it accepts the order, an already + // closed one would be registered in its terminal state and the events we replay for it dropped leanOrder.Status = OrderStatus.New; OnNewBrokerageOrderNotification(new NewBrokerageOrderNotificationEventArgs(leanOrder)); - if (leanOrder.Id == 0) { - // the algorithm's brokerage message handler didn't accept the order, so we won't track it return BrokerageSideOrderResult.NotAccepted; } + // cache the order with no execution progress before emitting anything: the regular fill detection diffs the + // real state against it, emitting the fills and the final status it might already have. Should we fail past + // this point, the fill polling picks up from here + var isClosed = OrderIsClosed(brokerageSideOrder); + var cachedOrder = new TradierCachedOpenOrder(new TradierOrder + { + Id = brokerageSideOrder.Id, + Direction = brokerageSideOrder.Direction, + RemainingQuantity = brokerageSideOrder.Quantity, + // we are about to report it as submitted, a live order keeps its status so we don't report a change that never happened + Status = isClosed ? TradierOrderStatus.Submitted : brokerageSideOrder.Status + }); + _cachedOpenOrdersByTradierOrderID[brokerageSideOrder.Id] = cachedOrder; + OnOrderEvent(new OrderEvent(leanOrder, DateTime.UtcNow, OrderFee.Zero, "Order was submitted outside Lean") { Status = OrderStatus.Submitted }); - // diff the order against a clone of itself with no execution progress, this way the regular fill - // detection emits the fills and the final status the order might already have - var brokerageSideOrderClone = brokerageSideOrder.Clone(); - brokerageSideOrderClone.AverageFillPrice = 0m; - brokerageSideOrderClone.QuantityExecuted = 0m; - brokerageSideOrderClone.LastFillPrice = 0m; - brokerageSideOrderClone.LastFillQuantity = 0m; - brokerageSideOrderClone.RemainingQuantity = brokerageSideOrder.Quantity; - if (OrderIsClosed(brokerageSideOrder)) + // Lean only applies fills carried by a fill status, so a closed order that executed part of its quantity + // has to report that fill before its final status + if (isClosed && brokerageSideOrder.Status != TradierOrderStatus.Filled && brokerageSideOrder.QuantityExecuted > 0) { - // we just reported the order as submitted, keeping the status of the orders that are still - // open avoids emitting an event for a status change that never happened - brokerageSideOrderClone.Status = TradierOrderStatus.Submitted; + var partiallyFilledOrder = new TradierOrder + { + Id = brokerageSideOrder.Id, + Direction = brokerageSideOrder.Direction, + Status = TradierOrderStatus.PartiallyFilled, + QuantityExecuted = brokerageSideOrder.QuantityExecuted, + RemainingQuantity = brokerageSideOrder.RemainingQuantity, + LastFillPrice = brokerageSideOrder.LastFillPrice + }; + ProcessPotentiallyUpdatedOrder(cachedOrder, partiallyFilledOrder); + cachedOrder.Order = partiallyFilledOrder; } - var cachedOrder = new TradierCachedOpenOrder(brokerageSideOrderClone); + // this drops the order from the cache if it's closed ProcessPotentiallyUpdatedOrder(cachedOrder, brokerageSideOrder); - - // only start tracking the order once it holds its real state, caching it any earlier would expose - // the clone to the fill polling, which would emit the fills we just emitted again - if (!OrderIsClosed(brokerageSideOrder)) + if (!isClosed) { cachedOrder.Order = brokerageSideOrder; - _cachedOpenOrdersByTradierOrderID[brokerageSideOrder.Id] = cachedOrder; } return BrokerageSideOrderResult.Tracked; } + /// + /// The outcome of offering an order placed outside of the algorithm to it + /// + private enum BrokerageSideOrderResult + { + /// + /// The algorithm took ownership of the order and we started tracking it + /// + Tracked, + + /// + /// The algorithm was offered the order and didn't accept it + /// + NotAccepted, + + /// + /// The order couldn't be offered to the algorithm, we failed to convert it into a Lean order + /// + Unprocessable + } + private void ProcessPotentiallyUpdatedOrder(TradierCachedOpenOrder cachedOrder, TradierOrder updatedOrder) { // check for fills or status changes, for either fire a fill event @@ -1587,6 +1645,12 @@ protected static bool IsShort(TradierOrderDirection direction) /// protected Order ConvertOrder(TradierOrder order) { + // Tradier reports these on the underlying, we would turn the whole strategy into a single order on it + if (order.Class is TradierOrderClass.Multileg or TradierOrderClass.Combo) + { + throw new NotSupportedException($"The Tradier order class {order.Class} is not supported."); + } + Order qcOrder; var symbol = _symbolMapper.GetLeanSymbol(order.Class == TradierOrderClass.Option ? order.OptionSymbol : order.Symbol); From 0fcab909e3e3a9aeccebfd2adec47f5278827879 Mon Sep 17 00:00:00 2001 From: Alexandre Catarino Date: Fri, 11 Sep 2026 23:40:45 +0100 Subject: [PATCH 09/13] Keep the callers that don't care about a failed orders fetch as they were Co-Authored-By: Claude Fable 5.1 --- .../TradierBrokerage.cs | 26 +++---------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/QuantConnect.TradierBrokerage/TradierBrokerage.cs b/QuantConnect.TradierBrokerage/TradierBrokerage.cs index a94b401..db9d42f 100644 --- a/QuantConnect.TradierBrokerage/TradierBrokerage.cs +++ b/QuantConnect.TradierBrokerage/TradierBrokerage.cs @@ -795,28 +795,13 @@ private bool TryDeserializeRemoveRoot(string json, string rootName, out T obj public override List GetOpenOrders() { var orders = new List(); + TryGetIntradayAndPendingOrders(out var openOrders); - if (!TryGetIntradayAndPendingOrders(out var intradayAndPendingOrders)) + foreach (var openOrder in openOrders.Where(OrderIsOpen)) { - return orders; - } - - var openOrders = intradayAndPendingOrders.Where(OrderIsOpen); - foreach (var openOrder in openOrders) - { - try - { - orders.Add(ConvertOrder(openOrder)); - } - catch (Exception err) - { - // an order Lean can't represent shouldn't keep the algorithm from launching, the fill polling reports it - Log.Error(err, $"skipping Tradier order {openOrder.Id}"); - continue; - } - // make sure our internal collection is up to date as well UpdateCachedOpenOrder(openOrder.Id, openOrder); + orders.Add(ConvertOrder(openOrder)); } return orders; @@ -1179,10 +1164,7 @@ out var rawContent { Task.Run(() => { - if (!TryGetIntradayAndPendingOrders(out var intradayAndPendingOrders)) - { - return; - } + TryGetIntradayAndPendingOrders(out var intradayAndPendingOrders); var orders = intradayAndPendingOrders .Where(x => x.Status == TradierOrderStatus.Rejected) .Where(x => DateTime.UtcNow - x.TransactionDate < TimeSpan.FromSeconds(2)); From e9e72fa69cdc5e20245cb526f95ba76f56cc857c Mon Sep 17 00:00:00 2001 From: Alexandre Catarino Date: Fri, 25 Sep 2026 18:24:58 +0100 Subject: [PATCH 10/13] Address review: public result enum, oto/oco/otoco classes, warn on unsupported orders An oto, oco or otoco order in the account failed the whole orders response to deserialize. Unsupported orders placed outside the algorithm now warn instead of stopping it, like IB. Co-Authored-By: Claude Opus 5.5 --- .../TradierBrokerageAditionalTests.cs | 83 ++++++++++-- QuantConnect.TradierBrokerage/Common.cs | 11 +- .../Models/Enums/BrokerageSideOrderResult.cs | 39 ++++++ .../TradierBrokerage.cs | 125 +++++++++--------- .../UnknownOrderIds.cs | 63 +++++++++ 5 files changed, 245 insertions(+), 76 deletions(-) create mode 100644 QuantConnect.TradierBrokerage/Models/Enums/BrokerageSideOrderResult.cs create mode 100644 QuantConnect.TradierBrokerage/UnknownOrderIds.cs diff --git a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs index 7a00d72..b9cdb08 100644 --- a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs +++ b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs @@ -363,7 +363,7 @@ public void NotifiesOrdersPlacedOutsideOfTheAlgorithm() orderProvider.Add(e.Order); }; - Assert.AreEqual("Tracked", + Assert.AreEqual(BrokerageSideOrderResult.Tracked, InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m))); Assert.IsNotNull(notifiedOrder); @@ -391,7 +391,7 @@ public void TracksOpenOrdersPlacedOutsideOfTheAlgorithmForFutureFills() brokerage.OrdersStatusChanged += (_, events) => orderEvents.AddRange(events); brokerage.NewBrokerageOrderNotification += (_, e) => orderProvider.Add(e.Order); - Assert.AreEqual("Tracked", InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Open))); + Assert.AreEqual(BrokerageSideOrderResult.Tracked, InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Open))); // the order is still open, so it's only reported as submitted, no fill event yet Assert.AreEqual(1, orderEvents.Count); @@ -414,7 +414,7 @@ public void ReportsThePartialFillOfClosedOrdersPlacedOutsideOfTheAlgorithm(Tradi brokerage.OrdersStatusChanged += (_, events) => orderEvents.AddRange(events); brokerage.NewBrokerageOrderNotification += (_, e) => orderProvider.Add(e.Order); - Assert.AreEqual("Tracked", + Assert.AreEqual(BrokerageSideOrderResult.Tracked, InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(status, quantityExecuted: 4m))); Assert.AreEqual(3, orderEvents.Count); @@ -440,7 +440,7 @@ public void DoesNotTrackOrdersPlacedOutsideOfTheAlgorithmWhenTheyAreNotAccepted( // the default brokerage message handler ignores these orders, leaving the Lean order id unset brokerage.NewBrokerageOrderNotification += (_, e) => notified = true; - Assert.AreEqual("NotAccepted", + Assert.AreEqual(BrokerageSideOrderResult.NotAccepted, InvokeHandleBrokerageSideOrder(brokerage, CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m))); Assert.IsTrue(notified); @@ -448,13 +448,16 @@ public void DoesNotTrackOrdersPlacedOutsideOfTheAlgorithmWhenTheyAreNotAccepted( Assert.IsFalse(GetCachedOpenOrders(brokerage).Contains(BrokerageSideOrderId)); } - // Orders LEAN cannot represent are never offered to the algorithm: the multileg only order types, and the - // multileg classes, which Tradier reports on the underlying and would otherwise convert to a single order on it + // Orders with legs aren't supported yet, so they're never offered to the algorithm: the multileg only order types + // and the classes that hold several orders as legs, which would otherwise convert to a single order [TestCase(TradierOrderType.Credit, TradierOrderClass.Multileg)] [TestCase(TradierOrderType.Debit, TradierOrderClass.Multileg)] [TestCase(TradierOrderType.Even, TradierOrderClass.Multileg)] [TestCase(TradierOrderType.Market, TradierOrderClass.Multileg)] [TestCase(TradierOrderType.Limit, TradierOrderClass.Combo)] + [TestCase(TradierOrderType.Limit, TradierOrderClass.Oto)] + [TestCase(TradierOrderType.Limit, TradierOrderClass.Oco)] + [TestCase(TradierOrderType.Limit, TradierOrderClass.Otoco)] public void ReportsUnsupportedOrdersAsUnprocessable(TradierOrderType type, TradierOrderClass orderClass) { var brokerage = CreateBrokerageWithOrderTracking(new OrderProvider()); @@ -466,13 +469,31 @@ public void ReportsUnsupportedOrdersAsUnprocessable(TradierOrderType type, Tradi brokerage.NewBrokerageOrderNotification += (_, e) => notified = true; var brokerageSideOrder = CreateBrokerageSideOrder(TradierOrderStatus.Open, type: type, orderClass: orderClass); - Assert.AreEqual("Unprocessable", InvokeHandleBrokerageSideOrder(brokerage, brokerageSideOrder)); + Assert.AreEqual(BrokerageSideOrderResult.Unprocessable, InvokeHandleBrokerageSideOrder(brokerage, brokerageSideOrder)); Assert.IsFalse(notified); Assert.IsEmpty(orderEvents); Assert.IsFalse(GetCachedOpenOrders(brokerage).Contains(BrokerageSideOrderId)); } + // An unknown class failed the whole orders response, so a single such order in the account broke the fill polling + [TestCase("oto", TradierOrderClass.Oto)] + [TestCase("oco", TradierOrderClass.Oco)] + [TestCase("otoco", TradierOrderClass.Otoco)] + public void DeserializesOrdersOfEveryClass(string orderClass, TradierOrderClass expected) + { + var json = $$$""" + {"orders":{"order":[{"id":{{{BrokerageSideOrderId}}},"status":"open","class":"{{{orderClass}}}","num_legs":2, + "leg":[{"id":1,"type":"limit","symbol":"SPY","side":"buy","quantity":1.0,"status":"open","duration":"day","price":1.0,"class":"equity"}, + {"id":2,"type":"stop","symbol":"SPY","side":"sell","quantity":1.0,"status":"open","duration":"day","stop_price":0.5,"class":"equity"}]}]}} + """; + + var orders = JsonConvert.DeserializeObject(json).Orders.Orders; + + Assert.AreEqual(1, orders.Count); + Assert.AreEqual(expected, orders[0].Class); + } + // Declining the order is a valid outcome, so the algorithm keeps running: it's warned once and the id is // marked as verified, else every poll would offer the order to the algorithm again [Test] @@ -516,10 +537,51 @@ public void WarnsOnceAboutOrdersPlacedOutsideOfTheAlgorithmThatItDoesNotAccept() // so the next poll neither flags it nor offers it again InvokeCheckForFills(brokerage); - Assert.IsEmpty(GetPrivateField>(typeof(TradierBrokerage), brokerage, "_unknownTradierOrderIDs")); + Assert.IsEmpty(GetPrivateField(typeof(TradierBrokerage), brokerage, "_unknownTradierOrderIDs").ToHashSet()); Assert.AreEqual(1, notifications); } + // Like a declined order, an unsupported one doesn't stop the algorithm, it's only warned about + [Test] + public void WarnsAboutUnsupportedOrdersPlacedOutsideOfTheAlgorithm() + { + var brokerageSideOrder = CreateBrokerageSideOrder(TradierOrderStatus.Open, orderClass: TradierOrderClass.Otoco); + // the order has to look newer than the brokerage instance for the fill polling to flag it + brokerageSideOrder.TransactionDate = DateTime.UtcNow.AddHours(1); + + var restClient = new Mock(); + restClient.Setup(x => x.Execute(It.IsAny())).Returns(() => CreateResponse(SerializeOrders(brokerageSideOrder))); + + var brokerage = CreateBrokerageWithOrderTracking(new OrderProvider(), restClient.Object); + + var notified = false; + brokerage.NewBrokerageOrderNotification += (_, e) => notified = true; + + List messages = []; + var warned = new ManualResetEvent(false); + brokerage.Message += (_, e) => + { + lock (messages) { messages.Add(e); } + if (e.Code == "UnprocessableOrderId") + { + warned.Set(); + } + }; + + InvokeCheckForFills(brokerage); + + Assert.IsTrue(warned.WaitOne(TimeSpan.FromSeconds(30)), + "The order was never reported. Messages: " + string.Join(" | ", messages.Select(x => $"{x.Type}:{x.Code}:{x.Message}"))); + lock (messages) + { + Assert.IsEmpty(messages.Where(x => x.Type == BrokerageMessageType.Error)); + var warning = messages.Single(x => x.Code == "UnprocessableOrderId"); + Assert.AreEqual(BrokerageMessageType.Warning, warning.Type); + Assert.IsTrue(warning.Message.Contains(BrokerageSideOrderId.ToStringInvariant()), warning.Message); + } + Assert.IsFalse(notified); + } + // A failed verification used to add the unknown ids back to the pending set, but a new verification task only // fires when that set is empty, so a single failure disabled the unknown order detection for the rest of the run [Test] @@ -617,10 +679,9 @@ private static IDictionary GetCachedOpenOrders(TradierBrokerage brokerage) return GetPrivateField(typeof(TradierBrokerage), brokerage, "_cachedOpenOrdersByTradierOrderID"); } - // the result type is private to the brokerage, so compare its name - private static string InvokeHandleBrokerageSideOrder(TradierBrokerage brokerage, TradierOrder brokerageSideOrder) + private static BrokerageSideOrderResult InvokeHandleBrokerageSideOrder(TradierBrokerage brokerage, TradierOrder brokerageSideOrder) { - return InvokePrivate(brokerage, "HandleBrokerageSideOrder", brokerageSideOrder).ToString(); + return (BrokerageSideOrderResult)InvokePrivate(brokerage, "HandleBrokerageSideOrder", brokerageSideOrder); } private static IRestResponse CreateResponse(string content, HttpStatusCode statusCode = HttpStatusCode.OK) diff --git a/QuantConnect.TradierBrokerage/Common.cs b/QuantConnect.TradierBrokerage/Common.cs index 448aedc..a552881 100644 --- a/QuantConnect.TradierBrokerage/Common.cs +++ b/QuantConnect.TradierBrokerage/Common.cs @@ -160,7 +160,16 @@ public enum TradierOrderClass Multileg, /// TradierOrderClass: Combo [EnumMember(Value = "combo")] - Combo + Combo, + /// TradierOrderClass: One triggers other + [EnumMember(Value = "oto")] + Oto, + /// TradierOrderClass: One cancels other + [EnumMember(Value = "oco")] + Oco, + /// TradierOrderClass: One triggers one cancels other + [EnumMember(Value = "otoco")] + Otoco } /// diff --git a/QuantConnect.TradierBrokerage/Models/Enums/BrokerageSideOrderResult.cs b/QuantConnect.TradierBrokerage/Models/Enums/BrokerageSideOrderResult.cs new file mode 100644 index 0000000..167e9e2 --- /dev/null +++ b/QuantConnect.TradierBrokerage/Models/Enums/BrokerageSideOrderResult.cs @@ -0,0 +1,39 @@ +/* + * QUANTCONNECT.COM - Democratizing Finance, Empowering Individuals. + * Lean Algorithmic Trading Engine v2.0. Copyright 2014 QuantConnect Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * +*/ + +namespace QuantConnect.Brokerages.Tradier +{ + /// + /// The outcome of offering an order placed outside of the algorithm to it + /// + public enum BrokerageSideOrderResult + { + /// + /// The algorithm took ownership of the order and we started tracking it + /// + Tracked, + + /// + /// The algorithm was offered the order and didn't accept it + /// + NotAccepted, + + /// + /// The order couldn't be offered to the algorithm, we failed to convert it into a Lean order + /// + Unprocessable + } +} diff --git a/QuantConnect.TradierBrokerage/TradierBrokerage.cs b/QuantConnect.TradierBrokerage/TradierBrokerage.cs index db9d42f..7a7fc85 100644 --- a/QuantConnect.TradierBrokerage/TradierBrokerage.cs +++ b/QuantConnect.TradierBrokerage/TradierBrokerage.cs @@ -110,7 +110,7 @@ public partial class TradierBrokerage : BaseWebsocketsBrokerage, IDataQueueHandl // this is used to block reentrance when handling contingent orders private readonly HashSet _contingentReentranceGuardByQCOrderID = new HashSet(); - private readonly HashSet _unknownTradierOrderIDs = new HashSet(); + private readonly UnknownOrderIds _unknownTradierOrderIDs = new(); private readonly FixedSizeHashQueue _verifiedUnknownTradierOrderIDs = new FixedSizeHashQueue(1000); private readonly FixedSizeHashQueue _cancelledQcOrderIDs = new FixedSizeHashQueue(10000); private string _restApiUrl = "https://api.tradier.com/v1/"; @@ -795,7 +795,10 @@ private bool TryDeserializeRemoveRoot(string json, string rootName, out T obj public override List GetOpenOrders() { var orders = new List(); - TryGetIntradayAndPendingOrders(out var openOrders); + if (!TryGetIntradayAndPendingOrders(out var openOrders)) + { + return orders; + } foreach (var openOrder in openOrders.Where(OrderIsOpen)) { @@ -1164,7 +1167,11 @@ out var rawContent { Task.Run(() => { - TryGetIntradayAndPendingOrders(out var intradayAndPendingOrders); + if (!TryGetIntradayAndPendingOrders(out var intradayAndPendingOrders)) + { + return; + } + var orders = intradayAndPendingOrders .Where(x => x.Status == TradierOrderStatus.Rejected) .Where(x => DateTime.UtcNow - x.TransactionDate < TimeSpan.FromSeconds(2)); @@ -1261,30 +1268,25 @@ private void CheckForFills() // if we get order updates for orders we're unaware of we need to bail, this can corrupt the algorithm state var unknownOrderIDs = updatedOrders.Where(IsUnknownOrderID).ToHashSet(x => x.Key); unknownOrderIDs.ExceptWith(_verifiedUnknownTradierOrderIDs); - bool fireTask; - lock (_unknownTradierOrderIDs) - { - // a pending id means a verification task is in flight, it releases them all once it's done - fireTask = unknownOrderIDs.Count != 0 && _unknownTradierOrderIDs.Count == 0; - _unknownTradierOrderIDs.UnionWith(unknownOrderIDs); - } - - if (fireTask) + if (_unknownTradierOrderIDs.TryBeginVerification(unknownOrderIDs)) { // wait a second and then check the order provider to see if we have these broker IDs, maybe they came in later (ex, symbol denied for short trading) Task.Delay(TimeSpan.FromSeconds(2)).ContinueWith(t => { - HashSet localUnknownTradierOrderIDs; - lock (_unknownTradierOrderIDs) - { - localUnknownTradierOrderIDs = [.. _unknownTradierOrderIDs]; - } + var localUnknownTradierOrderIDs = _unknownTradierOrderIDs.ToHashSet(); try { // verify we don't have them in the order provider Log.Trace("TradierBrokerage.CheckForFills(): Verifying missing brokerage IDs: " + string.Join(",", localUnknownTradierOrderIDs)); - var orders = localUnknownTradierOrderIDs.Select(x => _orderProvider?.GetOrdersByBrokerageId(x)?.SingleOrDefault()).Where(x => x != null); - var stillUnknownOrderIDs = localUnknownTradierOrderIDs.Where(x => !orders.Any(y => y.BrokerId.Contains(x.ToStringInvariant()))).ToList(); + List stillUnknownOrderIDs = []; + foreach (var unknownOrderID in localUnknownTradierOrderIDs) + { + var leanOrders = _orderProvider?.GetOrdersByBrokerageId(unknownOrderID); + if (leanOrders.IsNullOrEmpty()) + { + stillUnknownOrderIDs.Add(unknownOrderID); + } + } // without an order provider (data queue handler only mode) no order can be ours, // so don't treat orders placed externally in the account as an error if (_orderProvider != null && stillUnknownOrderIDs.Count > 0) @@ -1343,11 +1345,11 @@ private void CheckForFills() if (unprocessableOrderIDs.Count > 0) { - // we couldn't hand these orders over to the algorithm, so we've gotta bail on it + // like an order the algorithm declines, one we couldn't offer to it doesn't stop it, it's just not tracked var ids = string.Join(", ", unprocessableOrderIDs); Log.Error("TradierBrokerage.CheckForFills(): Unable to process the missing brokerage IDs: " + ids); - OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Error, "UnprocessableOrderId", - $"Tradier order id(s) {ids} were placed outside of the algorithm and cannot be represented as LEAN orders.")); + OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Warning, "UnprocessableOrderId", + $"Tradier order id(s) {ids} were placed outside of the algorithm and are not supported, so they will not be tracked.")); } } @@ -1366,10 +1368,7 @@ private void CheckForFills() } finally { - lock (_unknownTradierOrderIDs) - { - _unknownTradierOrderIDs.Clear(); - } + _unknownTradierOrderIDs.EndVerification(); } }); } @@ -1404,7 +1403,26 @@ private bool IsUnknownOrderID(KeyValuePair x) /// The outcome of offering the order to the algorithm private BrokerageSideOrderResult HandleBrokerageSideOrder(TradierOrder brokerageSideOrder) { - Order leanOrder; + if (!TryConvertBrokerageSideOrder(brokerageSideOrder, out var leanOrder)) + { + return BrokerageSideOrderResult.Unprocessable; + } + + // the transaction handler only normalizes a New status to Submitted when it accepts the order, an already + // closed one would be registered in its terminal state and the events we replay for it dropped + leanOrder.Status = OrderStatus.New; + OnNewBrokerageOrderNotification(new NewBrokerageOrderNotificationEventArgs(leanOrder)); + if (leanOrder.Id == 0) + { + return BrokerageSideOrderResult.NotAccepted; + } + + StartTrackingBrokerageSideOrder(brokerageSideOrder, leanOrder); + return BrokerageSideOrderResult.Tracked; + } + + private bool TryConvertBrokerageSideOrder(TradierOrder brokerageSideOrder, out Order leanOrder) + { try { leanOrder = ConvertOrder(brokerageSideOrder); @@ -1412,25 +1430,26 @@ private BrokerageSideOrderResult HandleBrokerageSideOrder(TradierOrder brokerage catch (Exception err) { Log.Error(err, $"failed to convert Tradier order {brokerageSideOrder.Id}"); - return BrokerageSideOrderResult.Unprocessable; + leanOrder = null; + return false; } - // GetOrder returns a blank order when the details request fails, we'd offer a stop at zero + // ConvertOrder reads the stop price with GetOrder, which returns a blank order when that request fails if (leanOrder is StopMarketOrder { StopPrice: 0 } or StopLimitOrder { StopPrice: 0 }) { - Log.Error($"TradierBrokerage.HandleBrokerageSideOrder(): could not fetch the stop price of Tradier order {brokerageSideOrder.Id}"); - return BrokerageSideOrderResult.Unprocessable; + Log.Error($"TradierBrokerage.TryConvertBrokerageSideOrder(): could not fetch the stop price of Tradier order {brokerageSideOrder.Id}"); + leanOrder = null; + return false; } - // the transaction handler only normalizes a New status to Submitted when it accepts the order, an already - // closed one would be registered in its terminal state and the events we replay for it dropped - leanOrder.Status = OrderStatus.New; - OnNewBrokerageOrderNotification(new NewBrokerageOrderNotificationEventArgs(leanOrder)); - if (leanOrder.Id == 0) - { - return BrokerageSideOrderResult.NotAccepted; - } + return true; + } + /// + /// Caches an order the algorithm accepted and emits its submission plus any fills and final status it already has + /// + private void StartTrackingBrokerageSideOrder(TradierOrder brokerageSideOrder, Order leanOrder) + { // cache the order with no execution progress before emitting anything: the regular fill detection diffs the // real state against it, emitting the fills and the final status it might already have. Should we fail past // this point, the fill polling picks up from here @@ -1471,29 +1490,6 @@ private BrokerageSideOrderResult HandleBrokerageSideOrder(TradierOrder brokerage { cachedOrder.Order = brokerageSideOrder; } - - return BrokerageSideOrderResult.Tracked; - } - - /// - /// The outcome of offering an order placed outside of the algorithm to it - /// - private enum BrokerageSideOrderResult - { - /// - /// The algorithm took ownership of the order and we started tracking it - /// - Tracked, - - /// - /// The algorithm was offered the order and didn't accept it - /// - NotAccepted, - - /// - /// The order couldn't be offered to the algorithm, we failed to convert it into a Lean order - /// - Unprocessable } private void ProcessPotentiallyUpdatedOrder(TradierCachedOpenOrder cachedOrder, TradierOrder updatedOrder) @@ -1627,8 +1623,9 @@ protected static bool IsShort(TradierOrderDirection direction) /// protected Order ConvertOrder(TradierOrder order) { - // Tradier reports these on the underlying, we would turn the whole strategy into a single order on it - if (order.Class is TradierOrderClass.Multileg or TradierOrderClass.Combo) + // these convert to several Lean orders, one per leg, which we don't track yet + if (order.Class is TradierOrderClass.Multileg or TradierOrderClass.Combo + or TradierOrderClass.Oto or TradierOrderClass.Oco or TradierOrderClass.Otoco) { throw new NotSupportedException($"The Tradier order class {order.Class} is not supported."); } diff --git a/QuantConnect.TradierBrokerage/UnknownOrderIds.cs b/QuantConnect.TradierBrokerage/UnknownOrderIds.cs new file mode 100644 index 0000000..7fc8e01 --- /dev/null +++ b/QuantConnect.TradierBrokerage/UnknownOrderIds.cs @@ -0,0 +1,63 @@ +/* + * QUANTCONNECT.COM - Democratizing Finance, Empowering Individuals. + * Lean Algorithmic Trading Engine v2.0. Copyright 2014 QuantConnect Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * +*/ + +using System.Collections.Generic; + +namespace QuantConnect.Brokerages.Tradier +{ + /// + /// The Tradier order ids the fill polling found and a verification task has to check, at most one task runs at a time + /// + public class UnknownOrderIds + { + private readonly HashSet _ids = []; + + /// + /// Adds the ids. Returns true when no task is in flight and there is something to verify, so the caller starts one + /// + public bool TryBeginVerification(IEnumerable ids) + { + lock (_ids) + { + var idle = _ids.Count == 0; + _ids.UnionWith(ids); + return idle && _ids.Count != 0; + } + } + + /// + /// The ids the running task has to verify + /// + public HashSet ToHashSet() + { + lock (_ids) + { + return [.. _ids]; + } + } + + /// + /// Releases all the ids once the task is done, so the next poll can start a new one + /// + public void EndVerification() + { + lock (_ids) + { + _ids.Clear(); + } + } + } +} From 96e6fe633e59787dfc45eb794990dd75dfb0be60 Mon Sep 17 00:00:00 2001 From: Alexandre Catarino Date: Fri, 25 Sep 2026 21:48:57 +0100 Subject: [PATCH 11/13] Skip unsupported open orders on launch with a warning An open multileg, combo, oto, oco or otoco order, or a stop whose price could not be fetched, made GetOpenOrders throw and failed the launch. Co-Authored-By: Claude Opus 5.5 --- .../TradierBrokerageAditionalTests.cs | 35 +++++++++++++++++++ .../TradierBrokerage.cs | 26 ++++++++++---- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs index b9cdb08..cef22e7 100644 --- a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs +++ b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs @@ -582,6 +582,41 @@ public void WarnsAboutUnsupportedOrdersPlacedOutsideOfTheAlgorithm() Assert.IsFalse(notified); } + // Open orders the plugin can't convert used to fail the launch, they're skipped with a warning instead + [Test] + public void SkipsUnsupportedOpenOrdersOnLaunch() + { + var supported = CreateBrokerageSideOrder(TradierOrderStatus.Open, type: TradierOrderType.Limit); + var unsupported = CreateBrokerageSideOrder(TradierOrderStatus.Open, orderClass: TradierOrderClass.Otoco); + unsupported.Id = BrokerageSideOrderId + 1; + // the stop price request returns no order, so its stop would be zero + var stopWithoutPrice = CreateBrokerageSideOrder(TradierOrderStatus.Open, type: TradierOrderType.StopMarket); + stopWithoutPrice.Id = BrokerageSideOrderId + 2; + + var restClient = new Mock(); + restClient.Setup(x => x.Execute(It.Is(r => r.Resource.EndsWith("/orders")))) + .Returns(() => CreateResponse(SerializeOrders(supported, unsupported, stopWithoutPrice))); + restClient.Setup(x => x.Execute(It.Is(r => !r.Resource.EndsWith("/orders")))) + .Returns(() => CreateResponse("{}")); + + var brokerage = CreateBrokerageWithOrderTracking(new OrderProvider(), restClient.Object); + SetPrivateField(typeof(TradierBrokerage), brokerage, "_accountId", "test"); + List messages = []; + brokerage.Message += (_, e) => messages.Add(e); + + var openOrders = brokerage.GetOpenOrders(); + + Assert.AreEqual(1, openOrders.Count); + CollectionAssert.AreEqual(new[] { BrokerageSideOrderId.ToStringInvariant() }, openOrders[0].BrokerId); + var warning = messages.Single(x => x.Code == "UnprocessableOrderId"); + Assert.AreEqual(BrokerageMessageType.Warning, warning.Type); + Assert.IsTrue(warning.Message.Contains($"{unsupported.Id}, {stopWithoutPrice.Id}"), warning.Message); + var cachedOpenOrders = GetCachedOpenOrders(brokerage); + Assert.IsTrue(cachedOpenOrders.Contains(supported.Id)); + Assert.IsFalse(cachedOpenOrders.Contains(unsupported.Id)); + Assert.IsFalse(cachedOpenOrders.Contains(stopWithoutPrice.Id)); + } + // A failed verification used to add the unknown ids back to the pending set, but a new verification task only // fires when that set is empty, so a single failure disabled the unknown order detection for the rest of the run [Test] diff --git a/QuantConnect.TradierBrokerage/TradierBrokerage.cs b/QuantConnect.TradierBrokerage/TradierBrokerage.cs index 7a7fc85..8ea11df 100644 --- a/QuantConnect.TradierBrokerage/TradierBrokerage.cs +++ b/QuantConnect.TradierBrokerage/TradierBrokerage.cs @@ -800,11 +800,25 @@ public override List GetOpenOrders() return orders; } + List unsupportedOrderIDs = []; foreach (var openOrder in openOrders.Where(OrderIsOpen)) { + // skipped orders stay out of the cache too, else the fill polling would look for a Lean order that doesn't exist + if (!TryConvertOrder(openOrder, out var leanOrder)) + { + unsupportedOrderIDs.Add(openOrder.Id); + continue; + } + // make sure our internal collection is up to date as well UpdateCachedOpenOrder(openOrder.Id, openOrder); - orders.Add(ConvertOrder(openOrder)); + orders.Add(leanOrder); + } + + if (unsupportedOrderIDs.Count > 0) + { + OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Warning, "UnprocessableOrderId", + $"Tradier open order id(s) {string.Join(", ", unsupportedOrderIDs)} are not supported, so they will not be tracked.")); } return orders; @@ -1403,7 +1417,7 @@ private bool IsUnknownOrderID(KeyValuePair x) /// The outcome of offering the order to the algorithm private BrokerageSideOrderResult HandleBrokerageSideOrder(TradierOrder brokerageSideOrder) { - if (!TryConvertBrokerageSideOrder(brokerageSideOrder, out var leanOrder)) + if (!TryConvertOrder(brokerageSideOrder, out var leanOrder)) { return BrokerageSideOrderResult.Unprocessable; } @@ -1421,15 +1435,15 @@ private BrokerageSideOrderResult HandleBrokerageSideOrder(TradierOrder brokerage return BrokerageSideOrderResult.Tracked; } - private bool TryConvertBrokerageSideOrder(TradierOrder brokerageSideOrder, out Order leanOrder) + private bool TryConvertOrder(TradierOrder order, out Order leanOrder) { try { - leanOrder = ConvertOrder(brokerageSideOrder); + leanOrder = ConvertOrder(order); } catch (Exception err) { - Log.Error(err, $"failed to convert Tradier order {brokerageSideOrder.Id}"); + Log.Error(err, $"failed to convert Tradier order {order.Id}"); leanOrder = null; return false; } @@ -1437,7 +1451,7 @@ private bool TryConvertBrokerageSideOrder(TradierOrder brokerageSideOrder, out O // ConvertOrder reads the stop price with GetOrder, which returns a blank order when that request fails if (leanOrder is StopMarketOrder { StopPrice: 0 } or StopLimitOrder { StopPrice: 0 }) { - Log.Error($"TradierBrokerage.TryConvertBrokerageSideOrder(): could not fetch the stop price of Tradier order {brokerageSideOrder.Id}"); + Log.Error($"TradierBrokerage.TryConvertOrder(): could not fetch the stop price of Tradier order {order.Id}"); leanOrder = null; return false; } From f58d342ef2e7b0d10477a0ea08d5e0c1614927d4 Mon Sep 17 00:00:00 2001 From: Alexandre Catarino Date: Mon, 28 Sep 2026 17:52:59 +0100 Subject: [PATCH 12/13] Address review: don't retry order placement, only log declined outside orders A timed out order POST can still reach Tradier, so retrying it could place the order twice and have the first one offered back as an outside order. Declined or unsupported outside orders are logged like the other brokerages do, a generic warning would belong in Lean. Co-Authored-By: Claude Opus 5.5 --- .../TradierBrokerageAditionalTests.cs | 76 +++++++++---------- .../TradierBrokerage.cs | 24 +++--- 2 files changed, 50 insertions(+), 50 deletions(-) diff --git a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs index cef22e7..7efaa41 100644 --- a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs +++ b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs @@ -130,6 +130,24 @@ public void RaisesErrorOnlyAfterRetriesAreExhaustedOnMalformedResponse() restClient.Verify(x => x.Execute(It.IsAny()), Times.Exactly(2)); } + // A timed out order POST can still reach Tradier, so retrying it could place the order twice. The failure + // doesn't stop the algorithm, TradierPlaceOrder invalidates the order and reports it + [Test] + public void DoesNotRetryOrderPlacement() + { + var errors = new List(); + var restClient = new Mock(); + restClient.Setup(x => x.Execute(It.IsAny())) + .Returns(() => CreateResponse("Bad Gateway", HttpStatusCode.BadGateway)); + + var brokerage = CreateBrokerageWithRestClient(restClient.Object, errors); + var result = InvokeExecute(brokerage, TradierApiRequestType.Orders, max: 3, resource: "accounts/test/orders", httpMethod: Method.POST); + + Assert.IsNull(result); + Assert.IsEmpty(errors); + restClient.Verify(x => x.Execute(It.IsAny()), Times.Once); + } + // Tradier's gateway can transiently serve a JSON fault body (e.g. {"fault":{"faultstring":"Datastore Error"}}) // for backend problems. Only non-retryable authentication faults may fail fast; everything else must take the // retry path (see https://github.com/QuantConnect/Lean.Brokerages.Tradier/issues/51). @@ -494,10 +512,10 @@ public void DeserializesOrdersOfEveryClass(string orderClass, TradierOrderClass Assert.AreEqual(expected, orders[0].Class); } - // Declining the order is a valid outcome, so the algorithm keeps running: it's warned once and the id is - // marked as verified, else every poll would offer the order to the algorithm again + // Declining the order is a valid outcome, so the algorithm keeps running without a message, like with the other + // brokerages. The id is marked as verified, else every poll would offer the order to the algorithm again [Test] - public void WarnsOnceAboutOrdersPlacedOutsideOfTheAlgorithmThatItDoesNotAccept() + public void IgnoresOrdersPlacedOutsideOfTheAlgorithmThatItDoesNotAccept() { var brokerageSideOrder = CreateBrokerageSideOrder(TradierOrderStatus.Filled, quantityExecuted: 10m); // the order has to look newer than the brokerage instance for the fill polling to flag it @@ -512,28 +530,17 @@ public void WarnsOnceAboutOrdersPlacedOutsideOfTheAlgorithmThatItDoesNotAccept() brokerage.NewBrokerageOrderNotification += (_, e) => Interlocked.Increment(ref notifications); List messages = []; - var warned = new ManualResetEvent(false); - brokerage.Message += (_, e) => - { - lock (messages) { messages.Add(e); } - if (e.Code == "UnknownOrderId") - { - warned.Set(); - } - }; + brokerage.Message += (_, e) => { lock (messages) { messages.Add(e); } }; InvokeCheckForFills(brokerage); - Assert.IsTrue(warned.WaitOne(TimeSpan.FromSeconds(30)), - "The order was never reported. Messages: " + string.Join(" | ", messages.Select(x => $"{x.Type}:{x.Code}:{x.Message}"))); - var warning = messages.Single(x => x.Code == "UnknownOrderId"); - Assert.AreEqual(BrokerageMessageType.Warning, warning.Type); - Assert.IsTrue(warning.Message.Contains(BrokerageSideOrderId.ToStringInvariant()), warning.Message); - Assert.AreEqual(1, notifications); - - // the id is verified right after the warning var verifiedOrderIDs = GetPrivateField>(typeof(TradierBrokerage), brokerage, "_verifiedUnknownTradierOrderIDs"); - Assert.IsTrue(SpinWait.SpinUntil(() => verifiedOrderIDs.Contains(BrokerageSideOrderId), TimeSpan.FromSeconds(5))); + Assert.IsTrue(SpinWait.SpinUntil(() => verifiedOrderIDs.Contains(BrokerageSideOrderId), TimeSpan.FromSeconds(30))); + lock (messages) + { + Assert.IsEmpty(messages, string.Join(" | ", messages.Select(x => $"{x.Type}:{x.Code}:{x.Message}"))); + } + Assert.AreEqual(1, notifications); // so the next poll neither flags it nor offers it again InvokeCheckForFills(brokerage); @@ -541,9 +548,9 @@ public void WarnsOnceAboutOrdersPlacedOutsideOfTheAlgorithmThatItDoesNotAccept() Assert.AreEqual(1, notifications); } - // Like a declined order, an unsupported one doesn't stop the algorithm, it's only warned about + // Like a declined order, an unsupported one doesn't stop the algorithm, it's only logged [Test] - public void WarnsAboutUnsupportedOrdersPlacedOutsideOfTheAlgorithm() + public void IgnoresUnsupportedOrdersPlacedOutsideOfTheAlgorithm() { var brokerageSideOrder = CreateBrokerageSideOrder(TradierOrderStatus.Open, orderClass: TradierOrderClass.Otoco); // the order has to look newer than the brokerage instance for the fill polling to flag it @@ -558,26 +565,15 @@ public void WarnsAboutUnsupportedOrdersPlacedOutsideOfTheAlgorithm() brokerage.NewBrokerageOrderNotification += (_, e) => notified = true; List messages = []; - var warned = new ManualResetEvent(false); - brokerage.Message += (_, e) => - { - lock (messages) { messages.Add(e); } - if (e.Code == "UnprocessableOrderId") - { - warned.Set(); - } - }; + brokerage.Message += (_, e) => { lock (messages) { messages.Add(e); } }; InvokeCheckForFills(brokerage); - Assert.IsTrue(warned.WaitOne(TimeSpan.FromSeconds(30)), - "The order was never reported. Messages: " + string.Join(" | ", messages.Select(x => $"{x.Type}:{x.Code}:{x.Message}"))); + var verifiedOrderIDs = GetPrivateField>(typeof(TradierBrokerage), brokerage, "_verifiedUnknownTradierOrderIDs"); + Assert.IsTrue(SpinWait.SpinUntil(() => verifiedOrderIDs.Contains(BrokerageSideOrderId), TimeSpan.FromSeconds(30))); lock (messages) { - Assert.IsEmpty(messages.Where(x => x.Type == BrokerageMessageType.Error)); - var warning = messages.Single(x => x.Code == "UnprocessableOrderId"); - Assert.AreEqual(BrokerageMessageType.Warning, warning.Type); - Assert.IsTrue(warning.Message.Contains(BrokerageSideOrderId.ToStringInvariant()), warning.Message); + Assert.IsEmpty(messages, string.Join(" | ", messages.Select(x => $"{x.Type}:{x.Code}:{x.Message}"))); } Assert.IsFalse(notified); } @@ -783,7 +779,7 @@ private static ManualResetEventSlim SignalFirstResponse(TradierBrokerage brokera return signal; } - private static T InvokeExecute(TradierBrokerage brokerage, TradierApiRequestType type, int max, string resource = "user/profile") where T : new() + private static T InvokeExecute(TradierBrokerage brokerage, TradierApiRequestType type, int max, string resource = "user/profile", Method httpMethod = Method.GET) where T : new() { var method = typeof(TradierBrokerage) .GetMethods(BindingFlags.NonPublic | BindingFlags.Instance) @@ -791,7 +787,7 @@ private static ManualResetEventSlim SignalFirstResponse(TradierBrokerage brokera .MakeGenericMethod(typeof(T)); try { - return (T)method.Invoke(brokerage, new object[] { new RestRequest(resource, Method.GET), type, "", max }); + return (T)method.Invoke(brokerage, new object[] { new RestRequest(resource, httpMethod), type, "", max }); } catch (TargetInvocationException e) { diff --git a/QuantConnect.TradierBrokerage/TradierBrokerage.cs b/QuantConnect.TradierBrokerage/TradierBrokerage.cs index 8ea11df..8b7fb75 100644 --- a/QuantConnect.TradierBrokerage/TradierBrokerage.cs +++ b/QuantConnect.TradierBrokerage/TradierBrokerage.cs @@ -176,6 +176,10 @@ public TradierBrokerage( var method = "TradierBrokerage.Execute." + request.Resource; var parameters = request.Parameters.Select(x => x.Name + ": " + x.Value); + // a failed order placement may still have reached Tradier, retrying it could place the order twice + var isOrderPlacement = request.Method == Method.POST && type == TradierApiRequestType.Orders; + max = isOrderPlacement ? 0 : max; + for (var attempt = 0; ; attempt++) { if (attempt != 0) @@ -251,6 +255,13 @@ public TradierBrokerage( Log.Trace(method + "(2): Attempting again..."); continue; } + + // TradierPlaceOrder invalidates the order and reports the failure, it doesn't have to stop the algorithm + if (isOrderPlacement) + { + return default(T); + } + // this body becomes the user's runtime error message: a proxy's HTML page is noise, log it only var detail = HtmlResponseRegex.IsMatch(raw.Content ?? string.Empty) ? "Tradier's API is likely temporarily unavailable, please contact support if it persists." @@ -1348,22 +1359,15 @@ private void CheckForFills() } } + // like the other brokerages, these are only logged: they aren't tracked and don't stop the algorithm if (notAcceptedOrderIDs.Count > 0) { - // declining an order placed outside of the algorithm is a valid outcome, we just won't track it - var ids = string.Join(", ", notAcceptedOrderIDs); - Log.Trace("TradierBrokerage.CheckForFills(): Orders placed outside of the algorithm were not accepted by it: " + ids); - OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Warning, "UnknownOrderId", - $"Tradier order id(s) {ids} were placed outside of the algorithm and were not accepted by its brokerage message handler, so they will not be tracked.")); + Log.Trace("TradierBrokerage.CheckForFills(): Orders placed outside of the algorithm were not accepted by it: " + string.Join(", ", notAcceptedOrderIDs)); } if (unprocessableOrderIDs.Count > 0) { - // like an order the algorithm declines, one we couldn't offer to it doesn't stop it, it's just not tracked - var ids = string.Join(", ", unprocessableOrderIDs); - Log.Error("TradierBrokerage.CheckForFills(): Unable to process the missing brokerage IDs: " + ids); - OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Warning, "UnprocessableOrderId", - $"Tradier order id(s) {ids} were placed outside of the algorithm and are not supported, so they will not be tracked.")); + Log.Error("TradierBrokerage.CheckForFills(): Unable to process the orders placed outside of the algorithm: " + string.Join(", ", unprocessableOrderIDs)); } } From 21ee7016229f1a35d5f794c7fd32e198cab7208d Mon Sep 17 00:00:00 2001 From: Alexandre Catarino Date: Mon, 28 Sep 2026 18:22:47 +0100 Subject: [PATCH 13/13] Address review: never offer the algorithm its own order, warn on unsupported ones A slow placement reply left the order visible to the poll before its id was on the Lean order, so it was offered back as an outside order. The verification now waits for in-flight placements and rechecks the cache and order provider under the fill lock before offering. Each offered id is verified as it's handled, so a failure later in the batch doesn't re-offer the ones already declined. Unsupported outside orders warn again in both paths, as Alpaca does; only declined ones are log-only. Co-Authored-By: Claude Fable 5.1 --- .../TradierBrokerageAditionalTests.cs | 141 +++++++++++++++++- .../TradierBrokerage.cs | 40 ++++- 2 files changed, 173 insertions(+), 8 deletions(-) diff --git a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs index 7efaa41..6c292a5 100644 --- a/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs +++ b/QuantConnect.TradierBrokerage.Tests/TradierBrokerageAditionalTests.cs @@ -548,9 +548,10 @@ public void IgnoresOrdersPlacedOutsideOfTheAlgorithmThatItDoesNotAccept() Assert.AreEqual(1, notifications); } - // Like a declined order, an unsupported one doesn't stop the algorithm, it's only logged + // Like a declined order, an unsupported one doesn't stop the algorithm. Unlike it, it's warned about: the account + // holds an order Lean can't see, as Alpaca does for its multi-leg orders [Test] - public void IgnoresUnsupportedOrdersPlacedOutsideOfTheAlgorithm() + public void WarnsAboutUnsupportedOrdersPlacedOutsideOfTheAlgorithm() { var brokerageSideOrder = CreateBrokerageSideOrder(TradierOrderStatus.Open, orderClass: TradierOrderClass.Otoco); // the order has to look newer than the brokerage instance for the fill polling to flag it @@ -565,19 +566,147 @@ public void IgnoresUnsupportedOrdersPlacedOutsideOfTheAlgorithm() brokerage.NewBrokerageOrderNotification += (_, e) => notified = true; List messages = []; - brokerage.Message += (_, e) => { lock (messages) { messages.Add(e); } }; + var warned = new ManualResetEvent(false); + brokerage.Message += (_, e) => + { + lock (messages) { messages.Add(e); } + if (e.Code == "UnprocessableOrderId") + { + warned.Set(); + } + }; InvokeCheckForFills(brokerage); - var verifiedOrderIDs = GetPrivateField>(typeof(TradierBrokerage), brokerage, "_verifiedUnknownTradierOrderIDs"); - Assert.IsTrue(SpinWait.SpinUntil(() => verifiedOrderIDs.Contains(BrokerageSideOrderId), TimeSpan.FromSeconds(30))); + Assert.IsTrue(warned.WaitOne(TimeSpan.FromSeconds(30)), + "The order was never reported. Messages: " + string.Join(" | ", messages.Select(x => $"{x.Type}:{x.Code}:{x.Message}"))); lock (messages) { - Assert.IsEmpty(messages, string.Join(" | ", messages.Select(x => $"{x.Type}:{x.Code}:{x.Message}"))); + Assert.IsEmpty(messages.Where(x => x.Type == BrokerageMessageType.Error)); + var warning = messages.Single(x => x.Code == "UnprocessableOrderId"); + Assert.AreEqual(BrokerageMessageType.Warning, warning.Type); + Assert.IsTrue(warning.Message.Contains(BrokerageSideOrderId.ToStringInvariant()), warning.Message); } Assert.IsFalse(notified); } + // The poll can see an order we're placing before its response is back, when its id isn't on the Lean order yet, + // so the verification waits for the placement instead of offering the algorithm its own order as an outside one + [Test] + public void DoesNotOfferOrdersWhileAPlacementIsInFlight() + { + var brokerageSideOrder = CreateBrokerageSideOrder(TradierOrderStatus.Open); + // the order has to look newer than the brokerage instance for the fill polling to flag it + brokerageSideOrder.TransactionDate = DateTime.UtcNow.AddHours(1); + + var restClient = new Mock(); + restClient.Setup(x => x.Execute(It.IsAny())).Returns(() => CreateResponse(SerializeOrders(brokerageSideOrder))); + + var brokerage = CreateBrokerageWithOrderTracking(new OrderProvider(), restClient.Object); + var notifications = 0; + brokerage.NewBrokerageOrderNotification += (_, e) => Interlocked.Increment(ref notifications); + + SetPrivateField(typeof(TradierBrokerage), brokerage, "_pendingOrderPlacements", 1); + var pendingOrderIDs = GetPrivateField(typeof(TradierBrokerage), brokerage, "_unknownTradierOrderIDs"); + InvokeCheckForFills(brokerage); + Assert.IsNotEmpty(pendingOrderIDs.ToHashSet()); + + // the verification task ends without offering the order and leaves its id unverified for the next poll + Assert.IsTrue(SpinWait.SpinUntil(() => pendingOrderIDs.ToHashSet().Count == 0, TimeSpan.FromSeconds(30))); + Assert.AreEqual(0, notifications); + var verifiedOrderIDs = GetPrivateField>(typeof(TradierBrokerage), brokerage, "_verifiedUnknownTradierOrderIDs"); + Assert.IsFalse(verifiedOrderIDs.Contains(BrokerageSideOrderId)); + + // once the placement is done the order is offered + SetPrivateField(typeof(TradierBrokerage), brokerage, "_pendingOrderPlacements", 0); + InvokeCheckForFills(brokerage); + Assert.IsTrue(SpinWait.SpinUntil(() => notifications == 1, TimeSpan.FromSeconds(30))); + } + + // A placement can finish between the poll flagging its id and the verification running two seconds later, + // the verification rechecks the cache so the algorithm's own order isn't offered back to it + [Test] + public void DoesNotOfferAnOrderCachedAfterItWasFlagged() + { + var brokerageSideOrder = CreateBrokerageSideOrder(TradierOrderStatus.Open); + // the order has to look newer than the brokerage instance for the fill polling to flag it + brokerageSideOrder.TransactionDate = DateTime.UtcNow.AddHours(1); + + var restClient = new Mock(); + restClient.Setup(x => x.Execute(It.IsAny())).Returns(() => CreateResponse(SerializeOrders(brokerageSideOrder))); + + var brokerage = CreateBrokerageWithOrderTracking(new OrderProvider(), restClient.Object); + var notifications = 0; + brokerage.NewBrokerageOrderNotification += (_, e) => Interlocked.Increment(ref notifications); + + var pendingOrderIDs = GetPrivateField(typeof(TradierBrokerage), brokerage, "_unknownTradierOrderIDs"); + InvokeCheckForFills(brokerage); + Assert.IsNotEmpty(pendingOrderIDs.ToHashSet()); + // the placement completes while the verification waits + InvokePrivate(brokerage, "UpdateCachedOpenOrder", BrokerageSideOrderId, brokerageSideOrder); + + Assert.IsTrue(SpinWait.SpinUntil(() => pendingOrderIDs.ToHashSet().Count == 0, TimeSpan.FromSeconds(30))); + Assert.AreEqual(0, notifications); + Assert.IsTrue(GetCachedOpenOrders(brokerage).Contains(BrokerageSideOrderId)); + } + + // The ids used to be marked as verified only once the whole batch was handled, so a failure on one order had + // the next verification offer the algorithm the orders it had already declined + [Test] + public void DoesNotOfferDeclinedOrdersAgainWhenALaterOneFails() + { + var declined = CreateBrokerageSideOrder(TradierOrderStatus.Open); + var failing = CreateBrokerageSideOrder(TradierOrderStatus.Open); + failing.Id = BrokerageSideOrderId + 1; + // the orders have to look newer than the brokerage instance for the fill polling to flag them + declined.TransactionDate = failing.TransactionDate = DateTime.UtcNow.AddHours(1); + + var restClient = new Mock(); + restClient.Setup(x => x.Execute(It.IsAny())).Returns(() => CreateResponse(SerializeOrders(declined, failing))); + + // the verification looks the ids up before and, under the fill lock, while offering the orders: fail the second lookup + // of the second order, after the first one was already offered + var orderProvider = new Mock(); + orderProvider.Setup(x => x.GetOrdersByBrokerageId(declined.Id.ToStringInvariant())).Returns([]); + orderProvider.SetupSequence(x => x.GetOrdersByBrokerageId(failing.Id.ToStringInvariant())) + .Returns([]) + .Throws(new Exception("Verification failed")) + .Returns([]) + .Returns([]); + var brokerage = CreateBrokerageWithOrderTracking(orderProvider.Object, restClient.Object); + + List offered = []; + brokerage.NewBrokerageOrderNotification += (_, e) => { lock (offered) { offered.Add(e.Order.BrokerId.Single()); } }; + var verificationFailed = new AutoResetEvent(false); + brokerage.Message += (_, e) => + { + if (e.Code == "UnknownIdResolution") + { + verificationFailed.Set(); + } + }; + + InvokeCheckForFills(brokerage); + Assert.IsTrue(verificationFailed.WaitOne(TimeSpan.FromSeconds(30)), "The verification never failed"); + + var verifiedOrderIDs = GetPrivateField>(typeof(TradierBrokerage), brokerage, "_verifiedUnknownTradierOrderIDs"); + Assert.IsTrue(verifiedOrderIDs.Contains(declined.Id)); + Assert.IsFalse(verifiedOrderIDs.Contains(failing.Id)); + + // the failed task releases the ids once it's done, keep polling like the fill timer does until the retry verifies the other order + var deadline = DateTime.UtcNow.AddSeconds(30); + while (!verifiedOrderIDs.Contains(failing.Id)) + { + Assert.Less(DateTime.UtcNow, deadline, "The verification was never retried"); + InvokeCheckForFills(brokerage); + Thread.Sleep(100); + } + lock (offered) + { + CollectionAssert.AreEqual(new[] { declined.Id.ToStringInvariant(), failing.Id.ToStringInvariant() }, offered); + } + } + // Open orders the plugin can't convert used to fail the launch, they're skipped with a warning instead [Test] public void SkipsUnsupportedOpenOrdersOnLaunch() diff --git a/QuantConnect.TradierBrokerage/TradierBrokerage.cs b/QuantConnect.TradierBrokerage/TradierBrokerage.cs index 8b7fb75..b31b251 100644 --- a/QuantConnect.TradierBrokerage/TradierBrokerage.cs +++ b/QuantConnect.TradierBrokerage/TradierBrokerage.cs @@ -111,6 +111,8 @@ public partial class TradierBrokerage : BaseWebsocketsBrokerage, IDataQueueHandl private readonly HashSet _contingentReentranceGuardByQCOrderID = new HashSet(); private readonly UnknownOrderIds _unknownTradierOrderIDs = new(); + // placements whose Tradier id isn't cached yet, the unknown order verification waits for them + private int _pendingOrderPlacements; private readonly FixedSizeHashQueue _verifiedUnknownTradierOrderIDs = new FixedSizeHashQueue(1000); private readonly FixedSizeHashQueue _cancelledQcOrderIDs = new FixedSizeHashQueue(10000); private string _restApiUrl = "https://api.tradier.com/v1/"; @@ -1108,6 +1110,20 @@ protected override void OnMessage(BrokerageMessageEvent e) /// A object indicating the result of the order placement. /// private TradierOrderResponse TradierPlaceOrder(TradierPlaceOrderRequest order, bool isSubmittedEvent = true) + { + // until the response is back and the id cached, the fill polling can't tell this order from one placed outside + Interlocked.Increment(ref _pendingOrderPlacements); + try + { + return TradierPlaceOrderAndCache(order, isSubmittedEvent); + } + finally + { + Interlocked.Decrement(ref _pendingOrderPlacements); + } + } + + private TradierOrderResponse TradierPlaceOrderAndCache(TradierPlaceOrderRequest order, bool isSubmittedEvent) { string stopLimit = string.Empty; if (order.Price != 0 || order.Stop != 0) @@ -1301,6 +1317,13 @@ private void CheckForFills() var localUnknownTradierOrderIDs = _unknownTradierOrderIDs.ToHashSet(); try { + // one of these can be an order we're placing, we only learn its id from the response + if (Volatile.Read(ref _pendingOrderPlacements) > 0) + { + Log.Trace("TradierBrokerage.CheckForFills(): Order placement in flight, the next poll rechecks the missing brokerage IDs: " + string.Join(",", localUnknownTradierOrderIDs)); + return; + } + // verify we don't have them in the order provider Log.Trace("TradierBrokerage.CheckForFills(): Verifying missing brokerage IDs: " + string.Join(",", localUnknownTradierOrderIDs)); List stillUnknownOrderIDs = []; @@ -1339,6 +1362,13 @@ private void CheckForFills() { foreach (var stillUnknownOrderID in stillUnknownOrderIDs) { + // a placement that finished since the poll flagged it has the id cached or on its Lean order by now + if (_cachedOpenOrdersByTradierOrderID.ContainsKey(stillUnknownOrderID) + || !_orderProvider.GetOrdersByBrokerageId(stillUnknownOrderID).IsNullOrEmpty()) + { + continue; + } + if (!unknownOrders.TryGetValue(stillUnknownOrderID, out var unknownOrder)) { // we don't have the details of the order, so we can't offer it to the algorithm @@ -1356,18 +1386,24 @@ private void CheckForFills() unprocessableOrderIDs.Add(stillUnknownOrderID); break; } + // verified as soon as it's offered: should a later order fail, this one isn't offered again + _verifiedUnknownTradierOrderIDs.Add(stillUnknownOrderID); } } - // like the other brokerages, these are only logged: they aren't tracked and don't stop the algorithm + // declining an order placed outside of the algorithm is a valid outcome, like the other brokerages we only log it if (notAcceptedOrderIDs.Count > 0) { Log.Trace("TradierBrokerage.CheckForFills(): Orders placed outside of the algorithm were not accepted by it: " + string.Join(", ", notAcceptedOrderIDs)); } + // an order we can't offer is worth a warning, the account holds an order Lean can't see; it doesn't stop the algorithm if (unprocessableOrderIDs.Count > 0) { - Log.Error("TradierBrokerage.CheckForFills(): Unable to process the orders placed outside of the algorithm: " + string.Join(", ", unprocessableOrderIDs)); + var ids = string.Join(", ", unprocessableOrderIDs); + Log.Error("TradierBrokerage.CheckForFills(): Unable to process the orders placed outside of the algorithm: " + ids); + OnMessage(new BrokerageMessageEvent(BrokerageMessageType.Warning, "UnprocessableOrderId", + $"Tradier order id(s) {ids} were placed outside of the algorithm and are not supported, so they will not be tracked.")); } }