Skip to content

QFJ-940 - Event log omits parts of received message in case of validation errors #172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 5, 2018
Merged

QFJ-940 - Event log omits parts of received message in case of validation errors #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b31b085
QFJ-940: Event log omits parts of received message in case of validat…
chrjohn Feb 2, 2018
4c4267f
QFJ-940: Event log omits parts of received message in case of validat…
chrjohn Feb 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions quickfixj-core/src/main/java/quickfix/Message.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ private Object cloneTo(Message message) {
* Do not call this method concurrently while modifying the contents of the message.
* This is likely to produce unexpected results or will fail with a ConcurrentModificationException
* since FieldMap.calculateString() is iterating over the TreeMap of fields.
*
* Use toRawString() to get the raw message data.
*
* @return Message as String with calculated body length and checksum.
*/
@Override
public String toString() {
Expand All @@ -144,6 +148,21 @@ public String toString() {
return sb.toString();
}

/**
* Return the raw message data as it was passed to the Message class.
*
* This is only available after Message has been parsed via constructor or Message.fromString().
* Otherwise this method will return NULL.
*
* This method neither does change fields nor calculate body length or checksum.
* Use toString() for that purpose.
*
* @return Message as String without recalculating body length and checksum.
*/
public String toRawString() {
return messageData;
}

public int bodyLength() {
return header.calculateLength() + calculateLength() + trailer.calculateLength();
}
Expand Down
45 changes: 24 additions & 21 deletions quickfixj-core/src/main/java/quickfix/Session.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,7 @@ private void next(Message message, boolean isProcessingQueuedMessages) throws Fi
// QFJ-650
if (!header.isSetField(MsgSeqNum.FIELD)) {
generateLogout("Received message without MsgSeqNum");
disconnect("Received message without MsgSeqNum: " + message, true);
disconnect("Received message without MsgSeqNum: " + getMessageToLog(message), true);
return;
}

Expand Down Expand Up @@ -975,7 +975,7 @@ private void next(Message message, boolean isProcessingQueuedMessages) throws Fi
if (rejectInvalidMessage) {
throw e;
} else {
getLog().onErrorEvent("Warn: incoming message with " + e + ": " + message);
getLog().onErrorEvent("Warn: incoming message with " + e + ": " + getMessageToLog(message));
}
} catch (final FieldException e) {
if (message.isSetField(e.getField())) {
Expand All @@ -984,22 +984,22 @@ private void next(Message message, boolean isProcessingQueuedMessages) throws Fi
} else {
getLog().onErrorEvent(
"Warn: incoming message with incorrect field: "
+ message.getField(e.getField()) + ": " + message);
+ message.getField(e.getField()) + ": " + getMessageToLog(message));
}
} else {
if (rejectInvalidMessage) {
throw e;
} else {
getLog().onErrorEvent(
"Warn: incoming message with missing field: " + e.getField()
+ ": " + e.getMessage() + ": " + message);
+ ": " + e.getMessage() + ": " + getMessageToLog(message));
}
}
} catch (final FieldNotFound e) {
if (rejectInvalidMessage) {
throw e;
} else {
getLog().onErrorEvent("Warn: incoming " + e + ": " + message);
getLog().onErrorEvent("Warn: incoming " + e + ": " + getMessageToLog(message));
}
}
}
Expand Down Expand Up @@ -1059,7 +1059,7 @@ private void next(Message message, boolean isProcessingQueuedMessages) throws Fi
ignore the message and let the problem correct itself (optimistic approach).
Target sequence number is not incremented, so it will trigger a ResendRequest
on the next message that is received. */
getLog().onErrorEvent("Skipping invalid message: " + e + ": " + message);
getLog().onErrorEvent("Skipping invalid message: " + e + ": " + getMessageToLog(message));
if (resetOrDisconnectIfRequired(message)) {
return;
}
Expand Down Expand Up @@ -1102,15 +1102,15 @@ ignore the message and let the problem correct itself (optimistic approach).
disconnect("Incorrect BeginString: " + e, true);
}
} catch (final IOException e) {
LogUtil.logThrowable(sessionID, "Error processing message: " + message, e);
LogUtil.logThrowable(sessionID, "Error processing message: " + getMessageToLog(message), e);
if (resetOrDisconnectIfRequired(message)) {
return;
}
} catch (Throwable t) { // QFJ-572
// If there are any other Throwables we might catch them here if desired.
// They were most probably thrown out of fromCallback().
if (rejectMessageOnUnhandledException) {
getLog().onErrorEvent("Rejecting message: " + t + ": " + message);
getLog().onErrorEvent("Rejecting message: " + t + ": " + getMessageToLog(message));
if (resetOrDisconnectIfRequired(message)) {
return;
}
Expand Down Expand Up @@ -1147,7 +1147,7 @@ private void handleExceptionAndRejectMessage(final String msgType, final Message
if (MsgType.LOGON.equals(msgType)) {
logoutWithErrorMessage(e.getMessage());
} else {
getLog().onErrorEvent("Rejecting invalid message: " + e + ": " + message);
getLog().onErrorEvent("Rejecting invalid message: " + e + ": " + getMessageToLog(message));
generateReject(message, e.getMessage(), e.getSessionRejectReason(), e.getField());
}
}
Expand All @@ -1162,7 +1162,7 @@ private void logoutWithErrorMessage(final String reason) throws IOException {
private boolean logErrorAndDisconnectIfRequired(final Exception e, Message message) {
final boolean resetOrDisconnectIfRequired = resetOrDisconnectIfRequired(message);
if (resetOrDisconnectIfRequired) {
getLog().onErrorEvent("Encountered invalid message: " + e + ": " + message);
getLog().onErrorEvent("Encountered invalid message: " + e + ": " + getMessageToLog(message));
}
return resetOrDisconnectIfRequired;
}
Expand Down Expand Up @@ -1323,7 +1323,7 @@ private void generateSequenceReset(Message receivedMessage, int beginSeqNo, int
receivedMessage.getHeader().getInt(MsgSeqNum.FIELD));
} catch (final FieldNotFound e) {
// should not happen as MsgSeqNum must be present
getLog().onErrorEvent("Received message without MsgSeqNum " + receivedMessage);
getLog().onErrorEvent("Received message without MsgSeqNum " + getMessageToLog(receivedMessage));
}
}
sendRaw(sequenceReset, beginSeqNo);
Expand Down Expand Up @@ -1508,7 +1508,7 @@ private void generateReject(Message message, String str) throws FieldNotFound, I

reject.setString(Text.FIELD, str);
sendRaw(reject, 0);
getLog().onErrorEvent("Reject sent for Message " + msgSeqNum + ": " + str);
getLog().onErrorEvent("Reject sent for message " + msgSeqNum + ": " + str);
}

private boolean isPossibleDuplicate(Message message) throws FieldNotFound {
Expand All @@ -1531,7 +1531,7 @@ private void generateReject(Message message, String text, int err, int field) th
}
if (!state.isLogonReceived()) {
final String errorMessage = "Tried to send a reject while not logged on: " + reason
+ " (field " + field + ")";
+ (reason.endsWith("" + field) ? "" : " (field " + field + ")");
throw new SessionException(errorMessage);
}

Expand Down Expand Up @@ -1588,16 +1588,15 @@ private void generateReject(Message message, String text, int err, int field) th
} finally {
state.unlockTargetMsgSeqNum();
}

final String logMessage = "Reject sent for message " + msgSeqNum;
if (reason != null && (field > 0 || err == SessionRejectReason.INVALID_TAG_NUMBER)) {
setRejectReason(reject, field, reason, true);
getLog().onErrorEvent(
"Reject sent for Message " + msgSeqNum + ": " + reason + ":" + field);
getLog().onErrorEvent(logMessage + ": " + reason + (reason.endsWith("" + field) ? "" : ":" + field));
} else if (reason != null) {
setRejectReason(reject, reason);
getLog().onErrorEvent("Reject sent for Message " + msgSeqNum + ": " + reason);
getLog().onErrorEvent(logMessage + ": " + reason);
} else {
getLog().onErrorEvent("Reject sent for Message " + msgSeqNum);
getLog().onErrorEvent(logMessage);
}

if (enableLastMsgSeqNumProcessed) {
Expand Down Expand Up @@ -1651,7 +1650,7 @@ private void generateBusinessReject(Message message, int err, int field) throws
final String reason = BusinessRejectReasonText.getMessage(err);
setRejectReason(reject, field, reason, field != 0);
getLog().onErrorEvent(
"Reject sent for Message " + msgSeqNum + (reason != null ? (": " + reason) : "")
"Reject sent for message " + msgSeqNum + (reason != null ? (": " + reason) : "")
+ (field != 0 ? (": tag=" + field) : ""));

sendRaw(reject, 0);
Expand Down Expand Up @@ -2283,7 +2282,7 @@ private void resendMessages(Message receivedMessage, int beginSeqNo, int endSeqN
if (begin != 0) {
generateSequenceReset(receivedMessage, begin, msgSeqNum);
}
getLog().onEvent("Resending Message: " + msgSeqNum);
getLog().onEvent("Resending message: " + msgSeqNum);
send(msg.toString());
begin = 0;
appMessageJustSent = true;
Expand Down Expand Up @@ -2577,7 +2576,7 @@ private boolean sendRaw(Message message, int num) {

return result;
} catch (final IOException e) {
logThrowable(getLog(), "Error Reading/Writing in MessageStore", e);
logThrowable(getLog(), "Error reading/writing in MessageStore", e);
return false;
} catch (final FieldNotFound e) {
logThrowable(state.getLog(), "Error accessing message fields", e);
Expand Down Expand Up @@ -2934,4 +2933,8 @@ private void resetIfSessionNotCurrent(SessionID sessionID, long time) throws IOE
}
}

private String getMessageToLog(final Message message) {
return (message.toRawString() != null ? message.toRawString() : message.toString());
}

}
16 changes: 16 additions & 0 deletions quickfixj-core/src/test/java/quickfix/MessageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1755,6 +1755,22 @@ public void testRepeatingGroupCountWithUnknownFields() throws Exception {
assertEquals("780508", underlyingSymbol);
}

@Test
// QFJ-940
public void testRawString() throws Exception {

String test = "8=FIX.4.4|9=431|35=d|49=1|34=2|52=20140117-18:20:26.629|56=3|57=21|322=388721|"
+ "323=4|320=1|393=42|82=1|67=1|711=1|311=780508|309=text|305=8|463=FXXXXX|307=text|542=20140716|"
+ "436=10.0|9013=1.0|9014=1.0|9017=10|9022=1|9024=1.0|9025=Y|916=20140701|917=20150731|9201=23974|"
+ "9200=17|9202=text|9300=727|9301=text|9302=text|9303=text|998=text|9100=text|9101=text|9085=text|"
+ "9083=0|9084=0|9061=579|9062=text|9063=text|9032=10.0|9002=F|9004=780415|9005=780503|10=223|";

DataDictionary dictionary = new DataDictionary(DataDictionaryTest.getDictionary());
Message message = new Message();
message.fromString(test.replaceAll("\\|", "\001"), dictionary, true);
assertEquals(test, message.toRawString().replaceAll("\001", "\\|"));
}

private void assertHeaderField(Message message, String expectedValue, int field)
throws FieldNotFound {
assertEquals(expectedValue, message.getHeader().getString(field));
Expand Down