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

E_NOTICE level error when updating additional owners. #521

Closed
karlr-stripe opened this issue Sep 17, 2018 · 2 comments · Fixed by #522
Closed

E_NOTICE level error when updating additional owners. #521

karlr-stripe opened this issue Sep 17, 2018 · 2 comments · Fixed by #522

Comments

@karlr-stripe
Copy link

I was following the instructions at https://stripe.com/docs/connect/identity-verification-api#additional-owners to update a connected account with a second additional owner. I noticed that the code actually raises a Undefined offset: 1 'error'. With default PHP settings, this is just a notice, and it isn't even logged.

The error seems to come from serializeAdditionalOwners:

stripe-php/lib/Account.php

Lines 229 to 237 in b078870

foreach ($additionalOwners as $i => $v) {
$update = ($v instanceof StripeObject) ? $v->serializeParameters() : $v;
if ($update !== []) {
if (!$originalValue || ($update != $legalEntity->serializeParamsValue($originalValue[$i], null, false, true))) {
$updateArr[$i] = $update;
}
}
}

$additionalOwners is the new value being passed in(which may be larger than the existing $originalValue), so we basically rely on being able to set an array property at an index that doesn't currently exist($originalValue[$i]). This works fine generally since it only throws an E_NOTICE level error.

However, if you have set up your PHP environment to convert errors into fatal exceptions(http://php.net/manual/en/class.errorexception.php#errorexception.examples), this can be a problem. I'm not sure how to proceed here since I'm not very familiar with PHP best practises — is it a problem that we generate an E_NOTICE level error and something that we should fix?

I have a repro script here : https://gist.github.com/karlr-stripe/a9447ef36d688afd84a5c1f3f9b294cf

You can workaround this by updating the account object this way instead :

$count = count($stripeAccount->legal_entity->additional_owners);
$newAdditionalOwner = [
    $count => [
      'first_name' => 'Jane',
      'last_name' => 'Smith',
    ]
  ];

  \Stripe\Account::update($stripeAccount->id, array(
    "legal_entity" => array(
      "additional_owners" => $newAdditionalOwner
    )
  ));
@ob-stripe
Copy link
Contributor

Hey @karlr-stripe, thanks for the report.

is it a problem that we generate an E_NOTICE level error and something that we should fix?

Yep, definitely.

This is probably my bad -- I ported the serialization logic over from stripe-ruby a while back, but Ruby simply returns a null value when trying to access invalid keys in a hash, without any warnings. I'll work on a fix.

@ob-stripe
Copy link
Contributor

Fixed in 6.17.2.

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

Successfully merging a pull request may close this issue.

2 participants