Closed ctlevi closed 6 months ago
Here is where the bug is happening:
@message
is nil because StandardError's constructor will not set that class property, we get the fallback message of "Error message: the server returns an error".Is there any reason that you aren't just letting the Faraday::TimeoutError be raised? I would prefer to deal with that.
cc: @vorpus
Just ran into this same issue today when upgrading the version of the plaid gem. I don't believe that Faraday::TimeoutError
should be raised in this case as that is an internal concern of this gem (@ctlevi). It seems like there should potentially another error class in this gem like ApiTimeoutError
that would be for this case. In the meantime I had to create a patch for this gem in order to do string comparison with the message of Connection timed out
.
module PlaidApiErrorPatch
def initialize(arg = nil)
arg.is_a?(String) ? super(message: arg) : super(arg)
end
end
Plaid::ApiError.prepend(PlaidApiErrorPatch)
Using this patch allows the message to be set correctly so that this will work:
[1] pry(main)> error = Plaid::ApiError.new('Connection timed out')
=> #<Plaid::ApiError: Connection timed out>
[2] pry(main)> error.message
=> "Connection timed out"
Hi @Maimer ! I hope all is well. My name is Gilberto and I’m on the Developer Relations team at Plaid. Our team is interested in expanding our Plaid community and we’re conducting a bit of research to learn more about how devs engage with us.
I was wondering if you'd be interested in participating in a 30 minute research session with me and a member of our research team? You would be compensated with a gift card for your time. If you’re interested, let me know and I can send over the Calendly invite so we can schedule some time (we could switch to email, Twitter DM, etc., for communication). Thanks! (Also, @ctlevi – the invite is also open to you as well in case you're interested 🙂 )
@otherchen Any ideas on this one? Do you think we should file an issue on the internal Jira?
Yeah, I think what @Maimer suggested - introducing a new timeout error type - seems like the right path forward! I'll create a JIRA to track this internally.
Is this issue resolved in later versions of the gem?
@kyledcline unfortunately I don't believe it's been resolved.
@kyledcline @phoenixy1 It hasn't been fixed in regards to introducing a new timeout error, but at least the string error message is preserved as of version 24.3 in this PR: https://github.com/plaid/plaid-ruby/pull/482, specifically this line: https://github.com/plaid/plaid-ruby/blob/master/lib/plaid/api_error.rb#L35. So if you were using the patch I was using earlier you no longer need it if you are using version 24.3 or newer:
[1] pry(main)> require 'plaid'
=> true
[2] pry(main)> error = Plaid::ApiError.new('Connection timed out')
=> #<Plaid::ApiError: Connection timed out>
[3] pry(main)> error.message
=> "Connection timed out"
[4] pry(main)> Plaid::VERSION
=> "24.3.0"
This is now resolved in the latest ruby client library. See https://github.com/plaid/plaid-ruby/pull/483
Version 13 and before allowed us to set a timeout for the client, and a
Faraday::TimeoutError
was raised. But in the latest version, 15.6.0, aPlaid::ApiError
is raised and all the fields are null. So I can't really tell if it is a timeout error, or a Plaid error.I would expect the timeout error that is raised to continue to be a
Faraday::TimeoutError
. Or at least for thePlaid::ApiError
to say that it is a client imposed timeout error.How to reproduce:
api_client.connection.options[:timeout] = 1
Plaid::ApiError
with all nil fields is raised