[QFJ-572] RuntimeExceptions cause messages to be silently dropped Created: 04/Feb/11  Updated: 14/Nov/12  Resolved: 14/Nov/12

Status: Resolved
Project: QuickFIX/J
Component/s: Engine
Affects Version/s: 1.5.0
Fix Version/s: 1.5.3

Type: Improvement Priority: Default
Reporter: Stelios Papadopoulos Assignee: Christoph John
Resolution: Fixed Votes: 2
Labels: None


 Description   

An Invalid argument exception was thrown from methods invoked in fromApp()

java.lang.IllegalArgumentException: message 8=FIXT.1.1^A9=480^A35=d^A34=7528 ...
at package.removed.ApplicationImpl.fromApp(ApplicationImpl.java:46)
at quickfix.Session.fromCallback(Session.java:1361)
at quickfix.Session.verify(Session.java:1314)
at quickfix.Session.verify(Session.java:1390)
at quickfix.Session.next(Session.java:796)
at quickfix.mina.SingleThreadedEventHandlingStrategy$SessionMessageEvent.processMessage(SingleThreadedEventHandlingStrategy.java:107)
at quickfix.mina.SingleThreadedEventHandlingStrategy.block(SingleThreadedEventHandlingStrategy.java:70)
at quickfix.mina.SingleThreadedEventHandlingStrategy$1.run(SingleThreadedEventHandlingStrategy.java:87)
at java.lang.Thread.run(Thread.java:619)

This caused QuickFixJ to think that it never received the message. So on the next message the sequence was out of order:

2011-02-04 07:41:21,737 [QFJ Message Processor] INFO quickfixj.event - FIXT.1.1:IGMM-MD->NDXMD: MsgSeqNum too high, expecting 7528 but received 7529

QuickFixJ then requested the message again.

The Exception was caused by the contents of the message, so on reception of the offending message the same thing happened again.

I believe that the default behaviour should be to reject the message indicating some kind of application failure, or something else other than silently dropping the message which can cause a resend loop.



 Comments   
Comment by Thomas Wölfle [ 20/Sep/11 ]

We had exactly the same problem. In our case in a Groovy Class, which was a subclass of ApplicationAdapter, an exception has been thrown that resulted in the described behaviour. In our case it was not a RuntimeException but a CheckedException. Since Groovy does not distinguish between runtime and checked exceptions the exception was simply thrown up the call stack resulting in the described behaviour above. I.e. perhaps not only RuntimeExceptions but all Throwables have to be catched in 'quickfix.Session.next'

Comment by Christoph John [ 20/Dec/11 ]

I think this would be a good enhancement. Basically, the behaviour is in-line with the spec which says:
"The receiving application should disregard any message that is garbled, cannot be parsed or fails a data integrity check. Processing of the next valid FIX message will cause detection of a sequence gap and a Resend Request will be generated. Logic should be included in the FIX engine to recognize the possible infinite resend loop, which may be encountered in this situation."

The tricky part here might be to discover an infinite resend loop. But as a first step it might be a good improvement to generate a Reject or BusinessMessageReject message if there is a problem inside Session.next(). This should be configurable (on/off).

Comment by Grant Birchmeier [ 09/Oct/12 ]

Given Christoph's quote of the spec, it sounds like QF/J is only lacking the loop detection, correct? Aside from that, it's behaving appropriately.

An option to generate a reject seems like merely a workaround, and not an actual solution.

I'm more interested in the actual reject cause - why the hell would FromApp() be throwing an IllegalArgumentException? That sounds like a bug somewhere. Same for Thomas' report – why would message-parsing throw a CheckedException? These are exceptions that imply coding problems.

Comment by Thomas Wölfle [ 09/Oct/12 ]

As described in my initial comment the ApplicationAdapter in our System can be extended by Groovy classes. We do this in order to be able to dynamically add code to patch incoming or outgoing FIX messages via Groovy scripts. In such a Groovy script it might happen that a Java method is called that throws a checked exception. Catching checked exceptions is verified by the Java compiler but since Groovy is a dynamic language there is no compilation that verifies that no such methods are called. This results in checked exceptions that are thrown but never caught.

Comment by Christoph John [ 10/Oct/12 ]

I can understand Grant's point as well as Thomas's.

Maybe the configurable rejection can be a workaround for the time being and we will add the infinite-loop-detection in a later version (suggestions on how to dectect this are welcome ).

Comment by Thomas Wölfle [ 11/Oct/12 ]

We have solved our problem using the following patch which simply adds another catch block in the method Session.next(Message). It is almost the same code as in the catch block for 'FieldNotFound' exceptions.

— core/src/main/java/quickfix/Session.java (revision 1052)
+++ core/src/main/java/quickfix/Session.java (working copy)
@@ -1023,6 +1023,23 @@
if (resetOrDisconnectIfRequired(message))

{ return; }

+ } catch(final QuickfixRuntimeException e) {
+ getLog().onErrorEvent("Rejecting message because of FrontFIX problem: " + e + ": " + message);
+ if (resetOrDisconnectIfRequired(message))

{ + return; + }

+ if (sessionID.getBeginString().compareTo(FixVersions.BEGINSTRING_FIX42) >= 0
+ && message.isApp())

{ + generateBusinessReject(message, + BusinessRejectReason.APPLICATION_NOT_AVAILABLE, 0); + }

else {
+ if (msgType.equals(MsgType.LOGON))

{ + getLog().onErrorEvent("Required field missing from logon"); + disconnect("Required field missing from logon", true); + }

else

{ + generateReject(message, "Application not available"); + }

+ }········
}
·
nextQueued();
@@ -2478,4 +2495,9 @@
this.checkGapFieldOnAdminMessage = checkGapFieldOnAdminMessage;
}
·
-}
\ No newline at end of file
+ public static class QuickfixRuntimeException extends RuntimeException {
+ public QuickfixRuntimeException(String message, Throwable cause)

{ + super(message, cause); + }

+ }
+}

Comment by Christoph John [ 11/Oct/12 ]

Thanks for your input, but I was more referring to a suggestion for the loop-detection.
I have already prepared a similar patch which catches Throwables and is configurable.

Comment by Christoph John [ 14/Nov/12 ]

Committed as rev #1101.

  • Introduced session setting RejectMessageOnUnhandledException.
  • If enabled, an uncaught Exception or Error in the application's message processing will lead to a (BusinessMessage)Reject being sent to the counterparty and the incoming message sequence number will be incremented.
  • If disabled (default), a quickfix.RuntimeError is thrown, the problematic incoming message is discarded and the message sequence number is not incremented. Processing of the next valid message will cause detection of a sequence gap and a ResendRequest will be generated.
Generated at Fri May 03 02:32:00 UTC 2024 using JIRA 7.5.2#75007-sha1:9f5725bb824792b3230a5d8716f0c13e296a3cae.