Exceptions

[1ber]

There is some reason to return null instead of raising exceptions in parser.get_message ?

[da4089]

When receiving messages via a socket connection, it's common to receive partial messages when under load. As these partial messages are appended to the reassembly buffer (using append_buffer, you can sometimes get no new FIX message from the appended bytes (and sometimes get multiple FIX messages).

So the recommended structure is to call get_message until it returns None.

I thought this was better than returning, eg. StopIteration, which would make the more usual case where you get one FIX message for each append_buffer kinda ugly.

That said, I'm playing with extensions to get_message that will do more (and optionally less) validation of the messages when parsing, and it would be possible to add an option to have it raise something rather than return None.

What's your use-case? Why is an exception the best solution for you? What exception would you expect?

[1ber]

I'm not in a real case scenario, i have already work with a big project with FIX, but in java.
I'm not an expert in python too, and i'm only playing with your lib. For me the advantage of exceptions would be a better description of the problem.
Don't get me wrong, i can be totally wrong, it's just that when i was playing with it, exceptions appears to me as a good idea in this scenario.
There is a lot of other things that can go wrong, apart from the message not being fully received. If everything return null there is no way to check what's wrong.
I pasted some sugestions bellow, but the best aprroach would be (i think) to have an Exception hierarchy that will give better clues of what is going wrong. Some pseudo-code/sugestions below:

` if len(self.pairs) == 0:
raise EmptySelfPairsException ( 'Empty pairs')

    # Check first pair is FIX BeginString.
    while self.pairs and self.pairs[0][0] != 8:
        # Discard pairs until we find the beginning of a message.
        self.pairs.pop(0)

    if len(self.pairs) == 0:
        raise EmptySelfPairsException ( 'Empty pairs')

    # Look for checksum.
    index = 0
    while index < len(self.pairs) and self.pairs[index][0] != 10:
        index += 1

    if index == len(self.pairs):
        raise ZeroIndexPairsException ( 'index==self.pairs')`

[da4089]

There is an interesting slippery slope here: the goal of a simple FIX library is to avoid some of the complexity of using, eg. QuickFIX. Once you start down the route of checking for tag 8 at the start, you end up then wanting to check tag 9 is next, and then tag 35, and then ... pretty soon you end up validating messages against a full spec/schema.

That said, the only way I currently check for message boundaries is tag 10, so ... there's at least a toe on that slope already.

I will think some more about this.

In the meantime, you should know that a returned None is not an error, it just means that no completed message has been found in the bytes accumulated so far. And you should normally call get_message repeatedly until it returns None when attempting to drain the accumulated buffer.

[1ber]

Hum, i understand you and recognize the merits of your view.
Should i close this issue?
Or you?
Tks

[da4089]

If you don't mind, I'd like to leave this open while I think some more. I'll update it once I've got a tentative plan, and see what you think.

[da4089]

Ok, here's my plan:

• I will add a bunch of behavioural settings to the parser
• These can be enabled or disabled either by calling FixParser.set_foo or passing foo=<bool> to the parser constructor
• One of these will be set_exceptions() (also, exceptions=True)

This setting will enable more detailed error detection and reporting, using a suitable set of Exception sub-classes defined for the various parsing errors.

I expect I'll have this done within a week or so.