moov-io / imagecashletter

X9’s Specifications for ICL (Image Cash Letter) to provide Check 21 services. The HTTP server is available in a Docker image and the Go package is available.
https://moov-io.github.io/imagecashletter/
Apache License 2.0
64 stars 39 forks source link

Errors thrown on X9 files from FRB #362

Closed smithbk closed 2 months ago

smithbk commented 5 months ago

What were you trying to do?

Read X9 files from the FRB

What did you expect to see?

When calling the Read method of an imagecashletter Reader to read an X9 file from the FRB, I would expect an option to more gracefully handle what the imagecashletter deems to be an invalid check field value. See "Additional comments/questions" for additional thoughts.

What did you see?

When calling the Read method on X9 files which we receive from the FRB, there are 3 types of errors which occur (or maybe more), preventing us from being able to process not only the check with the invalid field but also any of the checks in those X9 files. This is because it causes the Read method to return an error.

The specific errors being returned are as follows. I will be glad to split these into separate issues if needed.

  1. Invalid digital signature method The error message is as follows: "line:35321 record:ImageViewDetail *imagecashletter.FieldError DigitalSignatureMethod 0 is invalid" This means means that the FRB is sending a digital signature method value of "0" rather than "00". They also send the empty string value as well. I realize that according to the spec, it should be "00", but I still think having an option to be more tolerant would be good. The error is being returned at https://github.com/moov-io/imagecashletter/blob/master/imageViewDetail.go#L329

  2. Invalid image creator routing number field

    The error at https://github.com/moov-io/imagecashletter/blob/master/imageViewDetail.go#L367 is being returned because this field from the FRB is "000000000".

  3. BOFDEndorsementDate.IsZero()

    The error at https://github.com/moov-io/imagecashletter/blob/master/returnDetailAddendumA.go#L255 is being returned on some X9 files from the FRB.

We temporarily had to change the code to log those errors as warnings and then not return these errors.

How can we reproduce the problem?

I can not provide the X9 file from the FRB for security reasons, but I hope that by pointing to the specific error that is being returned, that will be enough.

Additional comments/questions

How should I handle this? Even if the FRB (or BOFD?) is setting incorrect field values according to the spec, it would be very helpful to provide an option to do one of the following. a) Allow imagecashletter users to provide their own per-field converter method. This way, I could provide my own code to convert a "0" digital signature method to "00". b) A configuration option to treat certain conditions as warnings rather than fatal errors which stop the processing of an X9 file altogether.

I would think that option "a" would be easier, safer, and more flexible for imagecashletter users. I would also be happy to submit a PR taking this general approach, but would prefer to hear from a maintainer first.

Thanks for any recommendations.

smithbk commented 5 months ago

@atonks2 Daniel said via slack: "I'm interested in the suggestion involving converters, but I'm curious how that would (or wouldn't) work with people using the codebase as a HTTP API vs importing as a lib."

Daniel, in order to make either method (a or b) work automatically for the REST service, I think it would require follow on work to what I'm proposing here. The simplest would be to do something similar to the following for the REST service, where the FRB compatibility mode is set via an environment variable:

if the FRB compatibility mode is enabled {
    set each of the FRB converter methods (e.g. "0" to "00")
}
smithbk commented 5 months ago

@atonks2 Daniel, I just received an update from the FRB and unfortunately it appears that there are are combination of types of changes that we need to make in order to be compatible with what we receive from them.

  1. Conversions - for the digital signature method and digital signature indicator fields, we need to convert the empty string or "0" to "00".
  2. Accept non-standard values: for the image creator route and BOFD endorsement date fields, we need to accept a string with all zeros.

Given that, I think the simplest and easiest-to-use change would be to simply add specific code to handle these 4 cases and guard this additional code with an "if FRB compatibility mode" check. This will then work for the HTTP API also, and if there are any future conversions or non-standard values which need to be accepted, we could just follow the same pattern.

If you agree with this approach, I'll submit a PR for it.

atonks2 commented 5 months ago

Thanks for confirming with the FRB @smithbk.

I agree with your suggestion and would be happy to review/accept a PR.

smithbk commented 5 months ago

@atonks2 Daniel, please see https://github.com/moov-io/imagecashletter/pull/364