moov-io / iso8583

A golang implementation to marshal and unmarshal iso8583 message.
https://moov.io
Apache License 2.0
304 stars 100 forks source link

Enable partial cloning of a Message #296

Closed meparle closed 7 months ago

meparle commented 8 months ago

Regarding Clone(): currently if unpacking one field fails, the whole unpacking process stops and returns an error, rather than returning a partially populated object along with any errors.

Currently this lets us know we need to respond with a format error, but it prevents the response being identifiable as a response to the message a sender has sent, because none of the fields that should be repeated back are present to enable them to match it.

If we had the partial message, then provided it had the minimum fields, we could respond with a format error and the sender would be able to match the response to the message they sent.

I propose additions to message.go, to allow a message to be partially populated. This could either be a new function, e.g. ClonePartial() or could be an update to Clone() that returns newMessage, err rather than either nil, err or newMessage, nil.

Note: The list of the minimum necessary fields to be able to respond to a particular message type can be defined outside of the library / remain out of scope.

Another thing that could be returned is a list of field numbers that errored during unpack() so that we don't bother trying them again in pack(); this and returning errors as an array are nice-to-haves. What's important is continuing to pack/unpack rather than stopping when erroring, and then returning the partially populated Message.

alovak commented 8 months ago

@meparle thank you for creating such a detailed explanation!

Here are some thoughts I want to share about the topic.

Parsing incorrectly formatted message

Can you provide some examples of parsing errors where the parser can still proceed with parsing subsequent fields?

In my experience, if a field cannot be parsed due to an error, the parser is often only able to read some of the subsequent fields when their length prefix is fixed. However, the values read might be incorrect. For instance, in the sequence 42840123456, the parser might interpret 428 as the currency code and 401234 as STAN. In reality, 840 is the currency code and 123456 is STAN. Hence, any fields that the parser reads after a parsing error are suspect.

Given this, we decided to halt parsing and return only the first error. This is because any subsequent errors might be misleading, as they could result from trying to parse misaligned data.

Notifying user about invalid message

Typically, errors related to message formatting arise during initial integrations. In production, they are rare. I understand that as a sender, if I send a message and the server cannot parse it, it won't be able to reply to my message. This results in a timeout, which can be misleading. Ideally, I'd like to receive an error response. However, given the parsing issue, most providers use alternate ways to notify users about invalid messages.

I've encountered several methods for notifying senders about these discrepancies:

  1. If the server cannot parse a message from the sender, it sends an Administrative message (0620) that contains both the error message and the original message to the sender.
  2. Some servers use message headers with specific flags. For instance, one flag might indicate that the message is a rejected message, and it shouldn't be treated as a request (since it will have a request MTI).

In both cases, the sender can parse the returned message, as it was originally generated by the sender. This allows them to extract details like STAN, RRN, etc., and match it to the pending request for further actions.

If you use the Moov's iso8583-connection package to manage connections, consider checking out this PR. It introduces a method (connection.RejectMessage) to handle such rejections. If you don't utilize Moov's package, you can still review the changes in the PR to see our approach to the rejected message issue.

meparle commented 7 months ago

Thanks very much for your analysis and suggestions @alovak - agree about 0620 messages to communicate with the acquirer, but as issuers we still need the partial message to respond intelligibly to the scheme with a format error.

I looked into it more and saw it was only unpack that needed changing for us at a minimum, for the smallest possible change. We can call Unpack() separately to calling Clone() and if there is an error we can respond.

I tried out continuing to parse after an error, and you are right that it can be done after a fixed length prefix, and is more challenging after the other prefix types.

Given this, and that having the partial message will be enough for our purposes, and that currently we do not have enough information to respond to the scheme without the partial message, I would like to return the partial message when there is a parsing error while unpacking.

I would also like to return the field number as a field in the UnpackError separate to the error message for convenience, as we would otherwise need to extract it.

My PR is here in thread in case you would like to see the code (my colleagues will also review it prior to me submitting it). Do let me know what you think of this as an approach.

alovak commented 7 months ago

Was implemented in #300