Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore null elements when autopaginating #1703

Merged
merged 1 commit into from
Jul 18, 2019
Merged

Conversation

ob-stripe
Copy link
Contributor

@ob-stripe ob-stripe commented Jul 10, 2019

r? @brandur-stripe
cc @stripe/api-libraries

Okay, so the bug here is a bit convoluted:

  • list elements in the data attribute of a list object are decoded using StripeObjectConverter:
    [JsonProperty("data", ItemConverterType = typeof(StripeObjectConverter))]
    public List<T> Data { get; set; }
  • when encountering an object it doesn't know how to decode, StripeObjectConverter returns null:
    // Couldn't find a concrete type to instantiate, return null.
    return null;

    (This was a deliberate design decision, because new object types may be added to the API and older versions of the library should not crash upon encountering them.)
  • the two above points mean that the Data list can contain null elements. It would be better if those null values were not added to the list at all, but unfortunately I don't think Newtonsoft.Json's ItemConverterType attribute lets us do that
  • when autopaginating, we cast elements to IHasId to extract the ID
  • of course, trying to access the ID on a null instance fails and throws a NullReferenceException

The fix is fairly simple: just skip over null elements in a page when autopaginating.

Also, for full context, I ran into this because stripe-mock returns a list element with object=alipay_account and the library does not have a model for this object type.

Copy link
Contributor

@brandur-stripe brandur-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks as usual for the detailed context!

@ob-stripe ob-stripe merged commit 7b4f3d1 into master Jul 18, 2019
@ob-stripe ob-stripe deleted the ob-auto-paging-skip-null branch July 18, 2019 17:34
@ob-stripe
Copy link
Contributor Author

Released as 27.14.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants