forwardfinancing/ach_client

Wrapper for various third party ACH services

ach_client's Issues

Test errors: undefined method 'new' and ArgumentError: wrong number of arguments

Running in an Ubuntu-based Docker container I had to apt-get install ruby-dev then I ran ./bin/setup.

Then I tried to run the tests

bundle exec rake test

I got a bunch of errors, showing only the first few.

There were several errors complaining about missing new and many about invalid number of arguments:

/var/lib/gems/3.0.0/gems/sucker_punch-2.1.2/lib/sucker_punch/testing/inline.rb:28: warning: method redefined; discarding old perform_async
/var/lib/gems/3.0.0/gems/sucker_punch-2.1.2/lib/sucker_punch/job.rb:35: warning: previous definition of perform_async was here
/var/lib/gems/3.0.0/gems/sucker_punch-2.1.2/lib/sucker_punch/testing/inline.rb:32: warning: method redefined; discarding old perform_in
/var/lib/gems/3.0.0/gems/sucker_punch-2.1.2/lib/sucker_punch/job.rb:41: warning: previous definition of perform_in was here
/var/lib/gems/3.0.0/gems/activesupport-5.2.8.1/lib/active_support/core_ext/hash/except.rb:12: warning: method redefined; discarding old except
/var/lib/gems/3.0.0/gems/mail-2.8.0/lib/mail/parsers/date_time_parser.rb:837: warning: statement not reached
/var/lib/gems/3.0.0/gems/mail-2.8.0/lib/mail/parsers/date_time_parser.rb:691: warning: assigned but unused variable - testEof
/opt/lib/ach_client.rb:8: warning: /opt/lib/ach_client.rb:8: warning: loading in progress, circular require considered harmful - /opt/lib/ach_client.rb
	from /usr/share/rubygems-integration/all/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:6:in  `<main>'
	from /usr/share/rubygems-integration/all/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:6:in  `select'
	from /usr/share/rubygems-integration/all/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:21:in  `block in <main>'
	from /usr/share/rubygems-integration/all/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:21:in  `require'
	from /opt/test/lib/ach_client/abstract/ach_batch_test.rb:1:in  `<top (required)>'
	from /opt/test/lib/ach_client/abstract/ach_batch_test.rb:1:in  `require'
	from /opt/test/test_helper.rb:17:in  `<top (required)>'
	from /opt/test/test_helper.rb:17:in  `require'
	from /opt/lib/ach_client.rb:7:in  `<top (required)>'
	from /opt/lib/ach_client.rb:7:in  `each'
	from /opt/lib/ach_client.rb:8:in  `block in <top (required)>'
	from /opt/lib/ach_client.rb:8:in  `require'

/var/lib/gems/3.0.0/gems/httpi-3.0.1/lib/httpi/request.rb:67: warning: Rack::Utils::HeaderHash is deprecated and will be removed in Rack 3.1, switch to Rack::Headers
/var/lib/gems/3.0.0/gems/httpi-3.0.1/lib/httpi/response.rb:23: warning: Rack::Utils::HeaderHash is deprecated and will be removed in Rack 3.1, switch to Rack::Headers
/var/lib/gems/3.0.0/gems/addressable-2.8.1/lib/addressable/uri.rb:417: warning: character class has duplicated range: /[^a-zA-Z0-9\-\.\_\~\!\$\&\'\(\)\*\+\,\;\=\:\@\[\:\]]/
/var/lib/gems/3.0.0/gems/httpi-3.0.1/lib/httpi/request.rb:67: warning: Rack::Utils::HeaderHash is deprecated and will be removed in Rack 3.1, switch to Rack::Headers
/var/lib/gems/3.0.0/gems/httpi-3.0.1/lib/httpi/response.rb:23: warning: Rack::Utils::HeaderHash is deprecated and will be removed in Rack 3.1, switch to Rack::Headers
/var/lib/gems/3.0.0/gems/addressable-2.8.1/lib/addressable/uri.rb:417: warning: character class has duplicated range: /[^a-zA-Z0-9\-\.\_\~\!\$\&\'\(\)\*\+\,\;\=\:\@\[\:\]]/
/opt/test/lib/ach_client/i_check_gateway/ach_status_checker_test.rb:51: warning: method redefined; discarding old test_in_range_rate_limit
/opt/test/lib/ach_client/i_check_gateway/ach_status_checker_test.rb:37: warning: previous definition of test_in_range_rate_limit was here
/opt/test/lib/ach_client/logging/logging_test.rb:34: warning: assigned but unused variable - output
/opt/test/lib/ach_client/logging/logging_test.rb:75: warning: assigned but unused variable - encrypted
/opt/test/lib/ach_client/logging/logging_test.rb:57: warning: method redefined; discarding old log_output
/opt/test/lib/ach_client/logging/logging_test.rb:33: warning: previous definition of log_output was here
Started with run options --seed 43684

ERROR SiliconValleyBank::AchTransactionTest#test_swaps_negative_credit_to_positive_debit (0.00s)
Minitest::UnexpectedError:         NoMethodError: undefined method `new' for BigDecimal:Class
            /opt/test/lib/ach_client/silicon_valley_bank/ach_transaction_test.rb:65:in `test_swaps_negative_credit_to_positive_debit'

ERROR SiliconValleyBank::AchTransactionTest#test_swaps_negative_debit_to_positive_credit (0.01s)
Minitest::UnexpectedError:         NoMethodError: undefined method `new' for BigDecimal:Class
            /opt/test/lib/ach_client/silicon_valley_bank/ach_transaction_test.rb:47:in `test_swaps_negative_debit_to_positive_credit'

 FAIL SiliconValleyBank::AchTransactionTest#test_send_abstract (0.01s)
        [RuntimeError] exception expected, not
        Class: <NoMethodError>
        Message: <"undefined method `new' for BigDecimal:Class">
        ---Backtrace---
        /opt/test/lib/ach_client/silicon_valley_bank/ach_transaction_test.rb:8:in `svb_transaction'
        /opt/test/lib/ach_client/silicon_valley_bank/ach_transaction_test.rb:37:in `block in test_send_abstract'
        ---------------
        /opt/test/lib/ach_client/silicon_valley_bank/ach_transaction_test.rb:36:in `test_send_abstract'

ERROR SiliconValleyBank::AchTransactionTest#test_that_it_works (0.01s)
Minitest::UnexpectedError:         NoMethodError: undefined method `new' for BigDecimal:Class
            /opt/test/lib/ach_client/silicon_valley_bank/ach_transaction_test.rb:8:in `svb_transaction'
            /opt/test/lib/ach_client/silicon_valley_bank/ach_transaction_test.rb:31:in `test_that_it_works'

ERROR SiliconValleyBank::AchBatchTest#test_override_file_creation_date (0.02s)
Minitest::UnexpectedError:         NoMethodError: undefined method `new' for BigDecimal:Class
            /opt/test/lib/ach_client/silicon_valley_bank/ach_batch_test.rb:9:in `transaction'
            /opt/test/lib/ach_client/silicon_valley_bank/ach_batch_test.rb:24:in `batch'
            /opt/test/lib/ach_client/silicon_valley_bank/ach_batch_test.rb:59:in `test_override_file_creation_date'

ERROR SiliconValleyBank::AchBatchTest#test_send_batch (0.02s)
Minitest::UnexpectedError:         NoMethodError: undefined method `new' for BigDecimal:Class
            /opt/test/lib/ach_client/silicon_valley_bank/ach_batch_test.rb:9:in `transaction'
            /opt/test/lib/ach_client/silicon_valley_bank/ach_batch_test.rb:24:in `batch'
            /opt/test/lib/ach_client/silicon_valley_bank/ach_batch_test.rb:41:in `block (2 levels) in test_send_batch'

ERROR SiliconValleyBank::AchBatchTest#test_nacha (0.02s)
Minitest::UnexpectedError:         NoMethodError: undefined method `new' for BigDecimal:Class
            /opt/test/lib/ach_client/silicon_valley_bank/ach_batch_test.rb:9:in `transaction'
            /opt/test/lib/ach_client/silicon_valley_bank/ach_batch_test.rb:24:in `batch'
            /opt/test/lib/ach_client/silicon_valley_bank/ach_batch_test.rb:52:in `test_nacha'

 FAIL SiliconValleyBank::AchBatchTest#test_send_invalid_batch (0.03s)
        [InvalidAchTransactionError] exception expected, not
        Class: <NoMethodError>
        Message: <"undefined method `new' for BigDecimal:Class">
        ---Backtrace---
        /opt/test/lib/ach_client/silicon_valley_bank/ach_batch_test.rb:9:in `transaction'
        /opt/test/lib/ach_client/silicon_valley_bank/ach_batch_test.rb:66:in `block in test_send_invalid_batch'
        ---------------
        /opt/test/lib/ach_client/silicon_valley_bank/ach_batch_test.rb:65:in `test_send_invalid_batch'

ERROR SiliconValleyBank::AchStatusCheckerTest#test_process_files_with_no_files (0.10s)
Minitest::UnexpectedError:         ArgumentError: wrong number of arguments (given 1, expected 0; required keywords: body, name)
            /opt/lib/ach_client/logging/log_provider_job.rb:10:in `perform'
            /var/lib/gems/3.0.0/gems/sucker_punch-2.1.2/lib/sucker_punch/testing/inline.rb:29:in `perform_async'
            /opt/lib/ach_client/providers/sftp/ach_status_checker.rb:113:in `block in files_in_range'
            /opt/lib/ach_client/providers/sftp/ach_status_checker.rb:111:in `map'
            /opt/lib/ach_client/providers/sftp/ach_status_checker.rb:111:in `files_in_range'
            /opt/lib/ach_client/providers/sftp/ach_status_checker.rb:152:in `block in most_recent_files'
            /opt/lib/ach_client/providers/sftp/sftp_provider.rb:45:in `block in with_sftp_connection'
            /var/lib/gems/3.0.0/gems/mocha-1.16.1/lib/mocha/invocation.rb:26:in `block in call'
            /var/lib/gems/3.0.0/gems/mocha-1.16.1/lib/mocha/invocation.rb:23:in `each'
            /var/lib/gems/3.0.0/gems/mocha-1.16.1/lib/mocha/invocation.rb:23:in `call'
            /var/lib/gems/3.0.0/gems/mocha-1.16.1/lib/mocha/expectation.rb:610:in `invoke'
            /var/lib/gems/3.0.0/gems/mocha-1.16.1/lib/mocha/mock.rb:321:in `handle_method_call'
            /var/lib/gems/3.0.0/gems/mocha-1.16.1/lib/mocha/stubbed_method.rb:60:in `block in define_new_method'
            /opt/lib/ach_client/providers/sftp/sftp_provider.rb:44:in `with_sftp_connection'
            /opt/lib/ach_client/providers/sftp/ach_status_checker.rb:151:in `most_recent_files'
            /opt/lib/ach_client/providers/sftp/ach_status_checker.rb:13:in `most_recent'
            /opt/test/lib/ach_client/silicon_valley_bank/ach_status_checker_test.rb:82:in `test_process_files_with_no_files'

# ruby --version
ruby 3.0.4p208 (2022-04-12 revision 3fa771dded) [x86_64-linux-gnu]

Test ticket from Code Climate

AchWorks issues

Discovered during production test:
originator_name - 16 char hard limit
memo - 10 char hard limit

Handle API rate limiting

ICheckGateway has an undocumented rate limit response case which looks like this:

<PullTransactionReportResponse xmlns="https://icheckgateway.com/API/">
  <PullTransactionReportResult>
     ACCESS DENIED: Report Call Limit Exceeded
  </PullTransactionReportResult>
</PullTransactionReportResponse>

The response HTTP status is still a 200, so no error is raised.
We should handle this response by raising an error

It seems the rate limit is unreasonably low - it was triggered for us by only ~20 requests in quick succession.
It looks like once the rate limit is reach, all requests fail for the rest of the day. Once they tell us what the rate limit is, we should write code to prevent it from happening, for example by raising an error before the final allowed request is made to avoid the ridiculous rest-of-day lockout

Namespace test files by provider

Right now multiple test classes have the same FQCN, which causes methods of the same name to override one another
Test classes should be namespaced by provider, following same structure as the lib file