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

Firestore : Protobuf error when transactionID is null #891

Closed
lemonlab opened this issue Feb 8, 2018 · 6 comments
Closed

Firestore : Protobuf error when transactionID is null #891

lemonlab opened this issue Feb 8, 2018 · 6 comments
Assignees
Labels
api: firestore Issues related to the Firestore API. type: question Request for information or clarification. Not an issue.

Comments

@lemonlab
Copy link

lemonlab commented Feb 8, 2018

When using the C implementation of protobuf, we can not use transactions in firestore, because protobuf is expecting a string for the transaction ID, and it may be null.

Exception :
User Error: Given value cannot be converted to string.

See : https://github.com/GoogleCloudPlatform/google-cloud-php/blob/master/src/Firestore/FirestoreClient.php#L438

    public function setRetryTransaction($var)
    {
        GPBUtil::checkString($var, False);
        $this->retry_transaction = $var;

        return $this;
    }

Steps to reproduce :

1 - Makes sure to use the C implementation of protobuf
pecl install protobuf
2 - Create a Firebase transaction

use Google\Cloud\Firestore\FirestoreClient;
use Google\Cloud\Firestore\Transaction;

$firestore = new FirestoreClient();
$document = $firestore->document('users/john');
$firestore->runTransaction(function (Transaction $transaction) use ($document) {
    // Manage Transaction.
});

An exception is raise even if the transaction contains no operation inside.

Stack trace

User Error: Given value cannot be converted to string.

  at vendor/google/proto-client/src/Google/Cloud/Firestore/V1beta1/TransactionOptions_ReadWrite.php:51
  at Google\Cloud\Firestore\V1beta1\TransactionOptions_ReadWrite->setRetryTransaction(null)
     (vendor/google/cloud-firestore/Connection/Grpc.php:111)
  at Google\Cloud\Firestore\Connection\Grpc->beginTransaction(array('database' => 'projects/project-XXXX/databases/(default)'))
     (vendor/google/cloud-firestore/FirestoreClient.php:454)
  at Google\Cloud\Firestore\FirestoreClient->Google\Cloud\Firestore\{closure}(object(Closure), array('maxRetries' => 5, 'begin' => array(), 'commit' => array(), 'rollback' => array()))
  at call_user_func_array(object(Closure), array(object(Closure), array('maxRetries' => 5, 'begin' => array(), 'commit' => array(), 'rollback' => array())))
     (vendor/google/cloud-core/Retry.php:78)
  at Google\Cloud\Core\Retry->execute(object(Closure), array(object(Closure), array('maxRetries' => 5, 'begin' => array(), 'commit' => array(), 'rollback' => array())))
     (vendor/google/cloud-firestore/FirestoreClient.php:485)
  at Google\Cloud\Firestore\FirestoreClient->runTransaction(object(Closure))
@jdpedrie jdpedrie added api: firestore Issues related to the Firestore API. type: question Request for information or clarification. Not an issue. labels Feb 8, 2018
@jdpedrie
Copy link
Contributor

jdpedrie commented Feb 8, 2018

Can you run php -v and pecl list and report the results?

@lemonlab
Copy link
Author

lemonlab commented Feb 8, 2018

php -v

PHP 7.1.13 (cli) (built: Jan  3 2018 17:31:04) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2017 Zend Technologies
    with Zend OPcache v7.1.13, Copyright (c) 1999-2017, by Zend Technologies

pecl list

Installed packages, channel pecl.php.net:
=========================================
Package  Version State
grpc     1.9.0   stable
imagick  3.4.3   stable
protobuf 3.5.1.1 stable

@jdpedrie
Copy link
Contributor

jdpedrie commented Feb 8, 2018

I'm having trouble triggering the error you've described. Can you try this and let me know if it fixes the issue? In src/Firestore/Connection/Grpc.php, replace beginTransaction() with the following:

    public function beginTransaction(array $args)
    {
        $rw = new TransactionOptions_ReadWrite;
        if ($retry = $this->pluck('retryTransaction', $args, false)) {
            $rw->setRetryTransaction($retry);
        }

        $args['options'] = new TransactionOptions;
        $args['options']->setReadWrite($rw);

        return $this->send([$this->firestore, 'beginTransaction'], [
            $this->pluck('database', $args),
            $this->addResourcePrefixHeader($args)
        ]);
    }

(Be sure you make this change in a clean copy of the library, WITHOUT the changes proposed in your pull request.)

@jdpedrie
Copy link
Contributor

@lemonlab any luck with the code I shared?

@lemonlab
Copy link
Author

Works perfectly, thank you !

@jdpedrie
Copy link
Contributor

Great! I'll get a bugfix in. Thanks for the report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the Firestore API. type: question Request for information or clarification. Not an issue.
Projects
None yet
Development

No branches or pull requests

3 participants