google-cloud-php icon indicating copy to clipboard operation
google-cloud-php copied to clipboard

Impossible to "fix" the `return_immediately` deprecation

Open pkruithof opened this issue 2 years ago • 1 comments

We are using PubSub heavily in our project, which means lots of long-lived consumers. Since streaming pulls are not a thing in PHP, we resort to an infinite while loop:

# simplified version of the code
while(true) {
  $this->processMessages(
    $subscription->pull()
  );
  sleep(1);
}

Because the return_immediately option has been deprecated some time ago, this produces a deprecation log on every pull request:

[2022-06-17T06:57:33.459907+00:00] deprecation.INFO: User Deprecated: return_immediately is deprecated. {"exception":"[object] (ErrorException(code: 0): User Deprecated: return_immediately is deprecated. at /app/vendor/google/cloud-pubsub/src/V1/PullRequest.php:138)"} []
[2022-06-17T06:57:33.460273+00:00] deprecation.INFO: User Deprecated: return_immediately is deprecated. {"exception":"[object] (ErrorException(code: 0): User Deprecated: return_immediately is deprecated. at /app/vendor/google/cloud-pubsub/src/V1/PullRequest.php:118)"} []
[2022-06-17T06:57:33.460353+00:00] deprecation.INFO: User Deprecated: return_immediately is deprecated. {"exception":"[object] (ErrorException(code: 0): User Deprecated: return_immediately is deprecated. at /app/vendor/google/cloud-pubsub/src/V1/PullRequest.php:118)"} []

I thought that with #4734 this would be solved, but unfortunately the deprecation is triggered in PullRequest::getReturnImmediately(), which does always get called by the internal protobuf Message class in some places.

At first I thought to submit a workaround for this, so that the deprecation is only shown when the option is set to true (false is the default):

    public function getReturnImmediately()
    {
        if ($this->return_immediately === true) {
            @trigger_error('return_immediately is deprecated.', E_USER_DEPRECATED);
        }
        return $this->return_immediately;
    }

The setter should still always trigger the deprecation, so you still see it when you use the option and set it to false.

However, then I noticed that this code is generated. So I don't think this is possible, unless there's a way to customize generated classes like this?

In any case, I would very much like to be able to remove this deprecation from our processes, as it's polluting our logs, making them harder to read. Is there another way to go here?

pkruithof avatar Jun 17 '22 07:06 pkruithof

Thank you for reporting this! I understand why this would be frustrating. I see two potential paths forward here:

  1. Track down what part of the stack calls getReturnImmediately(), and see if we can remove the call.
  2. Submit a PR to protocolbuffers/protobuf to remove the deprecated errors on the getters.

Simply editing the PullRequest.php file will not work, as these are autogenerated, and any change that isn't to the generator itself (e.g. the protobuf repository) would get overwritten.

Between the two options above, 1 is preferable.

bshaffer avatar Jun 17 '22 17:06 bshaffer

Hi @pkruithof I'm trying to replicated your issue with the following basic push and pull script, but I'm not getting any deprecation message as my code is never reaching the if block in the following script:

$pubsub = new PubSubClient();
$topic = $pubsub->topic('my-topic-name');
$subs = $pubsub->subscriber('my-subs-name');
$topic->publish($message = ['data' => 'Test Message']);
$messages = $subs->pull();
$subs->acknowledge($message[0]);

Can you check if there's a particular scenario in which you're getting the deprecation notice? Are you getting the notice for a basic script as above too?

yash30201 avatar Jul 25 '23 14:07 yash30201

Closing due to inactivity. Please reopen if the issue still persists.

yash30201 avatar Jul 31 '23 05:07 yash30201

Hi @yash30201, the deprecation is suppressed by the @ operator. You can see it when you enable the scream option in xdebug, or by adding this in your script for example:

set_error_handler(function ($errno, $errstr, $errfile, $errline) {
    throw new ErrorException($errstr, $errno, 1, $errfile, $errline);
});

result:

PHP Fatal error:  Uncaught ErrorException: return_immediately is deprecated. in /app/vendor/google/cloud-pubsub/src/V1/PullRequest.php:167
Stack trace:
#0 [internal function]: {closure}(16384, 'return_immediat...', '/app/vendor/goo...', 167)
#1 /app/vendor/google/cloud-pubsub/src/V1/PullRequest.php(167): trigger_error('return_immediat...', 16384)
#2 /app/vendor/google/protobuf/src/Google/Protobuf/Internal/Message.php(1598): Google\Cloud\PubSub\V1\PullRequest->getReturnImmediately()
#3 /app/vendor/google/protobuf/src/Google/Protobuf/Internal/Message.php(1842): Google\Protobuf\Internal\Message->existField(Object(Google\Protobuf\Internal\FieldDescriptor))
#4 /app/vendor/google/protobuf/src/Google/Protobuf/Internal/Message.php(1931): Google\Protobuf\Internal\Message->fieldByteSize(Object(Google\Protobuf\Internal\FieldDescriptor))
#5 /app/vendor/google/protobuf/src/Google/Protobuf/Internal/Message.php(1564): Google\Protobuf\Internal\Message->byteSize()
#6 /app/vendor/grpc/grpc/src/lib/AbstractCall.php(117): Google\Protobuf\Internal\Message->serializeToString()
#7 /app/vendor/grpc/grpc/src/lib/UnaryCall.php(39): Grpc\AbstractCall->_serializeMessage(Object(Google\Cloud\PubSub\V1\PullRequest))
#8 /app/vendor/grpc/grpc/src/lib/BaseStub.php(295): Grpc\UnaryCall->start(Object(Google\Cloud\PubSub\V1\PullRequest), Array, Array)
#9 /app/vendor/grpc/grpc/src/lib/BaseStub.php(545): Grpc\BaseStub->Grpc\{closure}('/google.pubsub....', Object(Google\Cloud\PubSub\V1\PullRequest), Array, Array, Array)
#10 /app/vendor/google/gax/src/Transport/GrpcTransport.php(218): Grpc\BaseStub->_simpleRequest('/google.pubsub....', Object(Google\Cloud\PubSub\V1\PullRequest), Array, Array, Array)
#11 /app/vendor/google/gax/src/GapicClientTrait.php(751): Google\ApiCore\Transport\GrpcTransport->startUnaryCall(Object(Google\ApiCore\Call), Array)
#12 /app/vendor/google/gax/src/Middleware/CredentialsWrapperMiddleware.php(59): Google\Cloud\PubSub\V1\Gapic\SubscriberGapicClient->Google\ApiCore\{closure}(Object(Google\ApiCore\Call), Array)
#13 /app/vendor/google/gax/src/Middleware/FixedHeaderMiddleware.php(68): Google\ApiCore\Middleware\CredentialsWrapperMiddleware->__invoke(Object(Google\ApiCore\Call), Array)
#14 /app/vendor/google/gax/src/Middleware/RetryMiddleware.php(85): Google\ApiCore\Middleware\FixedHeaderMiddleware->__invoke(Object(Google\ApiCore\Call), Array)
#15 /app/vendor/google/gax/src/Middleware/OptionsFilterMiddleware.php(62): Google\ApiCore\Middleware\RetryMiddleware->__invoke(Object(Google\ApiCore\Call), Array)
#16 /app/vendor/google/gax/src/GapicClientTrait.php(725): Google\ApiCore\Middleware\OptionsFilterMiddleware->__invoke(Object(Google\ApiCore\Call), Array)
#17 /app/vendor/google/cloud-pubsub/src/V1/Gapic/SubscriberGapicClient.php(1264): Google\Cloud\PubSub\V1\Gapic\SubscriberGapicClient->startCall('Pull', 'Google\\Cloud\\Pu...', Array, Object(Google\Cloud\PubSub\V1\PullRequest))
#18 [internal function]: Google\Cloud\PubSub\V1\Gapic\SubscriberGapicClient->pull('projects/dev-pk...', 1000, Array)
#19 /app/vendor/google/cloud-core/src/ExponentialBackoff.php(97): call_user_func_array(Array, Array)
#20 /app/vendor/google/cloud-core/src/GrpcRequestWrapper.php(148): Google\Cloud\Core\ExponentialBackoff->execute(Array, Array)
#21 /app/vendor/google/cloud-core/src/GrpcTrait.php(82): Google\Cloud\Core\GrpcRequestWrapper->send(Array, Array, Array)
#22 /app/vendor/google/cloud-pubsub/src/Connection/Grpc.php(411): Google\Cloud\PubSub\Connection\Grpc->send(Array, Array)
#23 /app/vendor/google/cloud-pubsub/src/Subscription.php(688): Google\Cloud\PubSub\Connection\Grpc->pull(Array)
#24 /app/bin/pubsub.php(18): Google\Cloud\PubSub\Subscription->pull()
#25 {main}
  thrown in /app/vendor/google/cloud-pubsub/src/V1/PullRequest.php on line 167

pkruithof avatar Aug 01 '23 13:08 pkruithof

@pkruithof as mentioned, this is an issue in protobuf, and should be handled over there. I filed this issue to hopefully track it: https://github.com/protocolbuffers/protobuf/issues/13428

Have you tried using the protobuf extension? It's possible that this problem won't exist there. Try installing the extension with pecl install protobuf.

bshaffer avatar Aug 01 '23 17:08 bshaffer

Have you tried using the protobuf extension? It's possible that this problem won't exist there. Try installing the extension with pecl install protobuf.

I think we had in the past, but removed it after we had some segfault issues in some minor release. Is the extension the preferred way to use it? I'm not familiar with the development of the extension vs the php implementation, if they're in sync and such. The extension will probably perform better I suspect. If you can elaborate a bit about it I would very much appreciate that.

pkruithof avatar Aug 02 '23 07:08 pkruithof