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

Recurly_Pager->current() breaks if the returned data is empty #354

Closed
arctouch-adrielvieira opened this issue Jun 14, 2018 · 2 comments · Fixed by #378
Closed

Recurly_Pager->current() breaks if the returned data is empty #354

arctouch-adrielvieira opened this issue Jun 14, 2018 · 2 comments · Fixed by #378
Assignees
Labels
V2 V2 Client

Comments

@arctouch-adrielvieira
Copy link

Hey all,

I'm facing an issue where Recurly throws an exception if my query does not find a matching record.

For example:

$subscriptions = Recurly_SubscriptionList::getForAccount($userAccountCode, ['state' => 'live']);

If the account exists but there's no live subscription, when calling $subscriptions->current(), the code will throw new Recurly_Error("Pager is not in a valid state");.

Here's the relevant code:

https://github.com/recurly/recurly-client-php/blob/master/lib/recurly/pager.php#L48-L60

This happens because sizeof($this->_objects)) is 0 (because there's no live subscriptions), and there's no next link.

This was changed in this commit, where this code was removed:

if ($this->count() == 0) {
    return null;
}

IMHO this still makes sense and should not be completely removed.

Any ideas on how to deal with this?

Thanks,

@nsanden
Copy link

nsanden commented Jun 28, 2018

Just try/catch it, unless I misunderstand the issue

@arctouch-adrielvieira
Copy link
Author

try/catch would work but it would also catch the times where this error happens for other reasons and not only because of the empty results.

A simple solution found later was to explicitly verify if $subscription->count() > 0 before calling->current(). But, IMO, the error message being thrown is not related to the real issue, which can make the users confused.

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

Successfully merging a pull request may close this issue.

3 participants