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

Compile Error: Declaration of Http\Message\Encoding\FilteredStream::seek(int $offset, int $whence = Http\Message\Encoding\SEEK_SET): void must be compatible with PsrExt\Http\Message\StreamInterface::seek($offset, $whence = NULL) #1718

Closed
codal-rthakkar opened this issue Mar 12, 2024 · 8 comments

Comments

@codal-rthakkar
Copy link

How do you use Sentry?

Sentry SaaS (sentry.io)

SDK version

3.22.1

Steps to reproduce

I didn't do anything, the same code was just working fine, but suddenly this error started popping up and 500 error started coming on my server,
whenever sentry tried to captureException or tried to send transaction.

i digged dipper and found out sentry endpoint was giving 429, i am not sure but might be because of this?

Here is the whole exception
Compile Error: Declaration of Http\Message\Encoding\FilteredStream::seek(int $offset, int $whence = Http\Message\Encoding\SEEK_SET): void must be compatible with PsrExt\Http\Message\StreamInterface::seek($offset, $whence = NULL)

Expected result

It should not break my code if issue is with sentry's code.

Actual result

It is breaking the whole code and server is responding with 500 error

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Mar 12, 2024
@Jean85
Copy link
Collaborator

Jean85 commented Mar 12, 2024

PsrExt is a strange namespace, is that maybe related to ext-psr? https://github.com/jbboehr/php-psr
I advise you to remove that extension, is not in sync with the proper PHP-FIG packages.

@codal-rthakkar
Copy link
Author

codal-rthakkar commented Mar 12, 2024

Psrext represents PSR extension installed as php module and yes it is https://github.com/jbboehr/php-psr, it is required because phalcon 4 needs it and the project is in phalcon4,
What i am more concerned about is, how the error started suddenly, because the same code and same modules were running perfectly before 7th march.

Here is phalcon4 link https://github.com/phalcon/cphalcon/tree/v4.1.2?tab=readme-ov-file

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Mar 12, 2024
@Jean85
Copy link
Collaborator

Jean85 commented Mar 12, 2024

You probably triggered a new codepath or updated some library. In particular, your error is that you're using an older ext-psr version that does not have types for its arguments (PsrExt\Http\Message\StreamInterface), while the implementation that lives in your vendor does (Http\Message\Encoding\FilteredStream).

You have to either upgrade the ext or downgrade the library.

I'm closing here since it's clear that it's not Sentry's fault, but feel free to continue replying if you need more details.

@codal-rthakkar
Copy link
Author

codal-rthakkar commented Mar 12, 2024

@Jean85 Thanks for the response,
I am well aware of the issue that function definition is incompatible,
But the same code with same package worked perfect till 7th march, and suddenly errors started coming in on 7th march,

Also i am not using the php-http/message package directly, it is dependency of sentry as you can see here
https://github.com/getsentry/sentry-php/blob/3.22.1/composer.json#L32

the error is showing in the file:vendor/php-http/message/src/Encoding/FilteredStream.php at line 178
which is file of the php-http/message package.
https://github.com/php-http/message/blob/1.x/src/Encoding/FilteredStream.php#L175

Now ideally it would extend the the Psr\Http\Message\StreamInterface class from psr module which is old,
but sentry has another package which overrides the dependency of psr module which is in "php-http/client-common"
https://github.com/getsentry/sentry-php/blob/3.22.1/composer.json#L29

now above package requires "psr/http-message" which overrides the name space of PSR module
here is the file: https://github.com/php-fig/http-message/blob/master/src/StreamInterface.php
now all of these packages are installed in my code base,
so ideally this should be extended from sentry code and not the namespace of php extension, and psr/http-message package has compatible function definition.

My main concern is still with how a perfectly running code of sentry, suddenly started throwing error,
might be the code was already there, but some response data changed which triggered that code to execute and we got this error.

thanks for the help.

@Jean85
Copy link
Collaborator

Jean85 commented Mar 12, 2024

The change could be in the 429 response from the server. That kind of response is a "Too Many Requests", so you're getting rejected, I don't know if you're making too many requests, if you're out of your paid quota or whatever.

The fact that the php-http packages is a sub dep of Sentry still your concern, not ours, since the fact that you're overriding the psr/http-message with an extension changes everything. You're basically getting this issue: jbboehr/php-psr#87

@codal-rthakkar
Copy link
Author

codal-rthakkar commented Mar 13, 2024

Hey @Jean85,
Just want to clear out that i don't want any changes from sentry side, i am just here to understand what went wrong and how i can avoid this kind of issues in future. because as i said it suddenly started coming in with the same code which was running in a container from 15 days.

One more question
Is this 429 response was changed around 7th march from sentry servers?
because for 200 response code sentry was working perfectly before and i guess earlier we were not getting 429 responses from sentry server.

Also JFI i have not explicitly override psr/http-message with psr extension it is how it is from 1.5 years in my code, and it was working perfectly fine.

thanks

@Jean85
Copy link
Collaborator

Jean85 commented Mar 13, 2024

I'll try to answer your doubts point by point:

  • you DO override psr/http-message, or well, the extension does the override for you, but only at the code level
  • you can't expect that your code stays ok if you do not upgrade your dependencies; in this case, the PSRs have went through a round of upgrades (for which I'm partially responsible, since I'm the author of the this bylaw) and the depending libraries (like php-http) have consequently adapted to it
  • in your specific case, you have PSR-7 psr/http-message as went through 1.1 (which adds types to method arguments) and 2.0, which adds method return types
  • your vendor folder seems to include psr/http-message 2.0, BUT the extension is providing the equivalent of 1.0, and this generates the error that you're reporting
  • so, you should declare the explicit replacement in the composer.json (as suggested in the extension readme) or at least declare it as an explicit require constraint (i.e. composer require psr/http-message ^1.0), so that Composer can avoid installing psr/http-message 2.0 (which is probably what's happening to you right now)
  • as for the 429 response, the Sentry docs explicitly says:

When you exceed your quota threshold, the server will respond with a 429 HTTP status code, which communicates to SDKs and clients to stop sending events. This status code comes with a Retry-After header that indicates the time for which this rate limit is active. However, clients are not supposed to retry events, but instead drop events until the rate limit has expired, to prevent queue backlogs. Note, that since event ingestion and rate limiting happen asynchronously, the 429 HTTP status code is always slightly delayed.

So, you have a quota configured on your account, and you're exceeding it.

@codal-rthakkar
Copy link
Author

Okay thanks @Jean85 for clearing out the doubts,
it makes sense that framework might be overriding the namespace,

I agree with you that older dependency would break sometimes,
but still my main doubt is how the same code which was running perfect in the same server environment, started getting the error 😄, there might be some change in sentry server architecture or response headers or something.
I guess i will not find out what exactly made the perfectly running code to break, but i will surely keep my dependencies updated from now on.

Also thanks for pointing the doc on php-psr repo, adding psr/http-message:1.0 fixes the issue,
But i will be upgrading sentry package to latest version.

Thanks for all the help and let me know if you get any information if there were any changes around 7th march.
🤝

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

No branches or pull requests

2 participants