[QFJ-954] sendRaw ignores return value of set call Created: 18/Sep/18 Updated: 18/Sep/18 |
|
| Status: | Open |
| Project: | QuickFIX/J |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Default |
| Reporter: | Philip Whitehouse | Assignee: | Unassigned |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Description |
|
The documentation for MessageStore.set() says:
* @return true is successful, false otherwise
* @throws IOException IO error
However the return code is pointless. set is called by SessionState.set which is in turned called by Session.sendRaw, which does the following:
if (num == 0) {
final int msgSeqNum = header.getInt(MsgSeqNum.FIELD);
if (persistMessages) {
state.set(msgSeqNum, messageString);
}
state.incrNextSenderMsgSeqNum();
}
return result;
So the return code is just pointless. Throwing an exception on the other hand, does cause the return value of sendRaw (and hence Session.send to change. Perhaps if it returns false Session.sendRaw should return false. (We may still want to call incrNextSenderMsgSeqNum however.) |
| Comments |
| Comment by Christoph John [ 18/Sep/18 ] |
|
I am not entirely sure but I might be that the return code is a relict from the C++ implementation. Here is the comment of the Session.send() method:
/**
* Send a message to a counterparty. Sequence numbers and information about the sender
* and target identification will be added automatically (or overwritten if that
* information already is present).
*
* The returned status flag is included for
* compatibility with the JNI API but it's usefulness is questionable.
* In QuickFIX/J, the message is transmitted using asynchronous network I/O so the boolean
* only indicates the message was successfully queued for transmission. An error could still
* occur before the message data is actually sent.
*
* @param message the message to send
* @return a status flag indicating whether the write to the network layer was successful.
*/
I think I already analyzed this some time ago. What I found more special was that the message is actually sent first and then persisted which might leave a little gap when there is a crash. The .NET and C++ implementations do persist first and send afterwards. Chris. |
| Comment by Philip Whitehouse [ 18/Sep/18 ] |
|
The comment about the return indicating a successful network write however is also broken however. In the case of set throwing an IOException instead of returning false it will return false despite having already written to the network. So it's definitely 'special'. |
| Comment by Christoph John [ 18/Sep/18 ] |
|
You could turn on synchronous writes if you want to make sure that the message was really sent. But as you said, the usage of the return code is somewhat inconsistent. |