Fix unmarshalling an expanded balance transaction source into a balan… #569
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks! Pulling this in.
Yeah, agreed. Unfortunately this is a fairly pervasive problem right now, so let's tackle that separately. |
|
Released as 32.0.1. |
brandur-stripe
pushed a commit
that referenced
this pull request
Jun 7, 2018
As described in #570, the current pattern in custom `UnmarshalJSON` implementations used to perform object expansion is quite poor because it assumes that *any* unmarshal error on an object indicates that the object was actually a string. This is not true, and these custom functions can have the effect of hiding actual problems and making debugging difficult (see #569). Here I introduce a new pattern for custom `UnmarshalJSON` implementations. Instead of trying for an object right away, it tries to detect whether the value is a string, and if it is, sets that as the object's ID. If it isn't, it then performs unmarshaling on the full object. If that errors, we return the error. This also has the benefit of making unmarshaling on values that are strings faster -- we return almost immediately instead of allocating an object, trying to decode it, failing, and falling back. I was hoping to be able to cut down the length of the implementation by quite a bit, and although it is shortened some, it's not too different. I'll also try to use this opportunity to standardize on conventions (name values `v`) and add some tests.
brandur-stripe
pushed a commit
that referenced
this pull request
Jun 11, 2018
As described in #570, the current pattern in custom `UnmarshalJSON` implementations used to perform object expansion is quite poor because it assumes that *any* unmarshal error on an object indicates that the object was actually a string. This is not true, and these custom functions can have the effect of hiding actual problems and making debugging difficult (see #569). Here I introduce a new pattern for custom `UnmarshalJSON` implementations. Instead of trying for an object right away, it tries to detect whether the value is a string, and if it is, sets that as the object's ID. If it isn't, it then performs unmarshaling on the full object. If that errors, we return the error. This also has the benefit of making unmarshaling on values that are strings faster -- we return almost immediately instead of allocating an object, trying to decode it, failing, and falling back. I was hoping to be able to cut down the length of the implementation by quite a bit, and although it is shortened some, it's not too different. I'll also try to use this opportunity to standardize on conventions (name values `v`) and add some tests.
nadaismail-stripe
pushed a commit
that referenced
this pull request
Oct 18, 2024
* Prorate backdated invoice phases * Refactor empty values are not allowed * Better test output formatting * Use date helpers to avoid date drift when running tests later in the day * Production packaging notes * Delete product2 objects too * Attempting to fix backdate test This is still failing, working in slack to resolve the issue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ce transaction
Currently it's trying to unmarshal into a string, which is causing an error.
Relatedly, in
(*BalanceTransaction) UnmarshalJSON(), there should be some handling of the unmarshal error beyond assuming that the error is for a particular reason, since that masked this issue, though not sure how y'all want to handle that.