Uploaded image for project: 'QuickFIX/J'
  1. QuickFIX/J
  2. QFJ-944

Misleading error message when receiving Logon with tag RawData without RawDataLength

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Default
    • Resolution: Fixed
    • Affects Version/s: 1.6.4
    • Fix Version/s: 2.0.1
    • Component/s: Engine, Metadata/Specs
    • Labels:
      None
    • Environment:
      Real connection to GDAX (Coinbase) exchange.

      Description

      I encountered a problem that after receiving a Logon message from the GDAX cryptocurrency exchange, the connection is breaking down with only one strange explaining string in event log messages:
      <2018-03-13 07:13:22.789 -0400> <Invalid LOGON message, disconnecting: Tag 95 not found in 8=FIX.4.2^A9=167^A35=A^A52=20180313-11:13:22.778^A49=Coinbase^A56=an_id^A34=1^A96=some_data^A98=0^A108=30^A554=password^A8013=Y^A10=032^A>
      It is obviously no tag 95 in the message, so from the first look it seems very strange that engine is trying to find it. As I found out after code debugging, the problem is that the tag 96 is defined as DATA in the standard FIX42 dictionary and for the type QuickFIX/J engine is looking for the previous tag: this logic is coded in the method quickfix.Message#extractField.
      I hotfixed my problem by changing the tag 96 type from DATA to STRING in the FIX dictionary, but it seems that it is too difficult to come to.

      1) I think, the FIX engine should check the previous tag presence before getting it, and in the case of absence, do extracting as with usual string.
      2) At any case, the log message should be more clear than simple "Tag <unknown tag> not found in <messageData>".

        Attachments

          Activity

          Hide
          chrjohn Christoph John added a comment -

          Hmm, tag 96 can contain any data, e.g. also SOH character. Not knowing the length of that tag can lead to parsing errors, especially on multi byte charsets.
          I am reluctant to change stuff which is not FIX compliant. Do they mention the reason of not using tag 95 somewhere?

          Show
          chrjohn Christoph John added a comment - Hmm, tag 96 can contain any data, e.g. also SOH character. Not knowing the length of that tag can lead to parsing errors, especially on multi byte charsets. I am reluctant to change stuff which is not FIX compliant. Do they mention the reason of not using tag 95 somewhere?
          Hide
          nikola Nikolai Kulakov added a comment - - edited

          No, they do not mention.
          You are right that not knowing the length of that tag can lead to parsing errors, but it is the fact that with current logic it is not working at all. So I think one of the ideas can be implemented:
          1) Write some warning/error message but try to parse as if it is a string;
          2) Introduce an option like "validateDataFieldsHaveLengthField" and try to parse if it is set to 'N';
          3) Leave the logic as it is, but write a more clear error message. I really thought that it is QuickFIX/J engine error that it is trying to get a tag that is not presented in the message.
          Thanks!

          Show
          nikola Nikolai Kulakov added a comment - - edited No, they do not mention. You are right that not knowing the length of that tag can lead to parsing errors, but it is the fact that with current logic it is not working at all. So I think one of the ideas can be implemented: 1) Write some warning/error message but try to parse as if it is a string; 2) Introduce an option like "validateDataFieldsHaveLengthField" and try to parse if it is set to 'N'; 3) Leave the logic as it is, but write a more clear error message. I really thought that it is QuickFIX/J engine error that it is trying to get a tag that is not presented in the message. Thanks!
          Hide
          chrjohn Christoph John added a comment -

          I think improving the error message is the only thing we can do at the moment. There is a workaround as you mentioned (changing the field to STRING). You could also set UseDataDictionary=N as long as GDAX does not use repeating groups.
          Feel free to open a separate issue or even a pull request for option 2.

          Thanks,
          Chris.

          Show
          chrjohn Christoph John added a comment - I think improving the error message is the only thing we can do at the moment. There is a workaround as you mentioned (changing the field to STRING). You could also set UseDataDictionary=N as long as GDAX does not use repeating groups. Feel free to open a separate issue or even a pull request for option 2. Thanks, Chris.
          Hide
          chrjohn Christoph John added a comment -

          What about this?

          throw new InvalidMessage("Did not find length field " + e.field + " required to parse data field " + tag + " in " + messageData);
          
          Show
          chrjohn Christoph John added a comment - What about this? throw new InvalidMessage( "Did not find length field " + e.field + " required to parse data field " + tag + " in " + messageData);
          Hide
          nikola Nikolai Kulakov added a comment -

          Yes, it is much clearer. Thanks!

          Show
          nikola Nikolai Kulakov added a comment - Yes, it is much clearer. Thanks!
          Show
          chrjohn Christoph John added a comment - https://github.com/quickfix-j/quickfixj/pull/181
          Hide
          chrjohn Christoph John added a comment -

          BTW, I have asked GDAX support if they can add the RawDataLength tag for better FIX compliance and they agreed to do so in the future.
          Until then please try with UseDataDictionary=N. They only seem to send one repeating group NoMiscFees with one instance as a response to the OrderStatusRequest. So not using the dictionary might very well work.

          Chris.

          Show
          chrjohn Christoph John added a comment - BTW, I have asked GDAX support if they can add the RawDataLength tag for better FIX compliance and they agreed to do so in the future. Until then please try with UseDataDictionary=N. They only seem to send one repeating group NoMiscFees with one instance as a response to the OrderStatusRequest. So not using the dictionary might very well work. Chris.

            People

            • Assignee:
              chrjohn Christoph John
              Reporter:
              nikola Nikolai Kulakov
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: