Missing Async support

[enumag]

At the moment, this library isn't fit to be used in async event loop environment such as Amphp or ReactPHP. While it internally uses asynchronous http client, the API itself is not asynchronous.

The HttpTransport calls wait() which ofc can't be used in an async event loop.

You could say that I can implement my own HttpTransport but the real problem is the TransportInterface. Which states that the send method should return either the event id or null on failure. In async environment I of course don't know if it failed or not since I can't wait for it so the interface is effectively impossible to fulfill. The best I can do is to return a Promise<string|null>.

By reading the documentation that this library isn't tied to any specific http client and that the used http client should be async I was hopeful that it might be fit to be used in async environment too. I'm very disappointed to see that it's actually not the case.

[Jean85]

This is unfortunately a long standing issue. The fact that PHP is not async as it is, makes this hard to implement.

Returning Promise<string|null> is probably the way to go; wait() was required by other issues that would make the SDK misbehave in more common situations, see #811 and following #813 #885 #981

[enumag]

Returning Promise<string|null> is probably the way to go

Unfortunately the interface doesn't allow me to do that. I don't really care if the wait is there for the default implementation as it is admittedly much more common use case. My point is that currently the interface itself doesn't allow an async implementation.

[ste93cry]

My point is that currently the interface itself doesn't allow an async implementation

Unfortunately by design the Unified API allows supporting an asyncronous API under the hood but regardless is meant to not expose any sign of it to the public. There is a lot of debate on this, and for what is worth I agree that returning Promise<string|null> would be the a great thing and that I would really like to see it happen. However, for the moment, there are no plans to make this happen, or at least not before version 3.0. Also because the Unified API applies to all languages supported by Sentry, there must be some kind of general consensus about new user-facing APIs that don't depend by me or the other contributors of this library. In the meanwhile you can follow getsentry/sentry-javascript#2049 for a similar request

[enumag]

Is someone who is in charge of designing this so-called "Unified API" aware of this serious drawback? Is it being worked on? I of course agree that this kind of change needs a new major release. So what? Can we make some progress towards it?

[ste93cry]

Is someone who is in charge of designing this so-called "Unified API" aware of this serious drawback?

There are similar issues here and there in various SDK repositories (e.g. the one I referenced above), so I'm pretty sure the Sentry guys do know this drawback. About making progress towards it, I don't think that anything will happen in the short time seeing the huge amount of time that passed since the opening of the first issue about this and the replies in it, I'm sorry

[prolic]

@enumag in amphp you can create a worker pool and push those sentry requests to the pool where they are executed synchronously. Using a pool allows to no wait for another request to be finished first. It's not the very best solution, but you would do the same for example to communicate with mongodb (because they don't have async interfaces for PHP either).

[rhcarvalho]

Looking at the TransportInterface

sentry-php/src/Transport/TransportInterface.php

Line 22 in f2b11fe

* @return string|null Returns the ID of the event or `null` if it failed to be sent

Another way to go about it is to reword Returns [...] 'null' if it failed to be sent to eliminate that requirement.

Of course, that's a semantic change and would be a silently breaking change. I'm not familiar with the implementation here to assess the impact of doing this in user code though. It might be perfectly fine to have a custom transport that never returns event IDs.

[tornadotwins]

One temporary way around this problem is the following:

Spin off a php file in a child-process ("exec php filename.php") that talks to Sentry. When the code execution of the child-process is completed, flush will be called on the library to submit the data to Sentry. Since it's contained in a different process, all is done without blocking your mainloop.

For ReactPHP, Clue's Peek library does this (returns the results as a promise):
https://github.com/clue/reactphp-pq
(Just contact him to get access to this lib).

The downside of course is that you loose the stack-trace since the request is executed in a different process altogether. So it requires being really descriptive in your errors/warning. But, it does work in a non-blocking fashion.

[ste93cry]

This should be possible since 3.0 thanks to the fact that the transport returns a promise. The promise will still be resolved while calling the capture* methods and the Sentry guys are not willing to change this, so if you need a full asyncronous client you will have to create your own implementation that uses the transport without calling wait() on the promise object.

[prolic]

I have a blog post on how I made sentry work with amphp: https://www.sasaprolic.com/2020/10/sentry-and-amphp.html

[drmax24]

This is unfortunately a long standing issue. The fact that PHP is not async as it is, makes this hard to implement.

curl_multi_init
works asynchronous

sentry php can actually cause your project to go down via blocking on external operations with this crap sentry library