search

plaid / plaid-ruby

Ruby bindings for Plaid
https://plaid.com/docs
MIT License
225 stars 143 forks source link

Gracefully handle bodies which are invalid from the JSON perspective #447

Closed olzv closed 1 year ago

olzv commented 1 year ago

In case a body contains any malformed JSON which can be even a blank string the code will raise a generic and not wrap the ApiError. Which then leads to inconsistent handling of the error on the gem users' side.

In example in case of the blank string in the body the gem will raise: JSON::ParserError: 859: unexpected token at ''

phoenixy1 commented 1 year ago

@olzv thanks for the PR!!

Looking at the header comments for this file, I am pretty sure they are indicating it is an auto-generated file that can't be edited directly (I mean, you technically can edit it directly, but those edits will be overwritten). I am not on the team that owns the client library generation process, but I think the edits need to be made in the template files instead at https://github.com/plaid/plaid-ruby/tree/master/templates/ruby -- can you take a look and see if that seems right / makes sense? If it's not obvious how to make the edit you're trying to make in the template files, let me know, and I can pull in someone from the owning team to take a look at it.

javierjulio commented 1 year ago

@olzv based on what @phoenixy1 shared, I think this is the file and method from the templates repo that would require this change.

olzv commented 1 year ago

@phoenixy1 @javierjulio I see your point. Yeah, we should change the template. Let me have a look.

olzv commented 1 year ago

@javierjulio @phoenixy1 I've pushed a change to align the template as well. Please have a look if that is what you meant. Thanks.

phoenixy1 commented 1 year ago

@olzv @javierjulio thank you for the PR! I just submitted it to upstream and it's currently in code review, hopefully we can merge it + incorporate it soon.

phoenixy1 commented 1 year ago

following up to indicate that this has been merged into upstream master and will be incorporated in the next client library release for ruby