sentry-symfony
sentry-symfony copied to clipboard
Improve test-command and developer-experience
Hi, we just stumbled across a wrong message in the sentry:test
command. As it turned out, the php-configuration on that server was missing the cacert.pem-file containing root-certificates to verify certificates sent by webservers.
However, the command told us
$ php bin/console sentry:test -vvv
DSN correctly configured in the current client
Sending test message...
Message sent successfully with ID xxx
Thats not correct - there was no message sent, since curl fails in the background with SSL certificate error: unable to get local issuer certificate
. So we searched for hours in the firewall and outbound-connection assuming the problem is there somewhere, since sentry told that the message was successully sent.
That should be changed, that the command really tests setup and connection and not only the attempt to send a message (or it should be clarified in the message).
Thanks!
The behavior of the HTTP layer is unfortunately out of our control. Maybe switching to Guzzle would make those errors bubble up?
BTW these errors would surface better with https://github.com/getsentry/sentry-php/pull/989, which is slated to be released as 2.4.
I have the same issue. sentry:test
command is not really usefull when there is delayed sending at the end of processing.
It shows message was sucessully sent when it was just added to the queue.
If there is such debug command - it should not use delay dispatching. As this is not showing real problems like ssl certificate problems.
@Jean85 it is not about HTTP layer behaviour.
It is about HttpTransport.php in Sentry which is not really even trying to send message when there is delay setting:
Problem is this is also used for test command which just add message to pendingRequests without even trying to send it - but shows it was successfully send!
Took me hours to debug that in reality I have SSL peer certificate or SSH remote key was not OK
error and test command was showing everything is fine.
Ouch, I forgot about the delayed sending. In the next major version that will be no longer the default behaviour, but we need to fix it here. Maybe we can use flush()
to force it to send the event?
@ste93cry is this still needed with 4.0?
The client still returns null from the capture*()
methods without giving any hint about why it failed to send the event. We now support a PSR-3 logger instance to debug such things, but it may still be worth to improve the command's UX. However, I tried to do that for the 4.0
milestone and I didn't find a good way nor I have an idea of what we could do to improve the situation as my opinion is that the command is not really useful because sending of an event through the client does not guarantee that all the things that would act on a normal "sending flow" are executed, so if the purpose of this command is to aid debugging then it's not really reliable
This issue has gone three weeks without activity. In another week, I will close it.
But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog
or Status: In Progress
, I will leave it alone ... forever!
"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀
I'm putting this on the backlog because I know that the DX on Laravel is far better than our, so it makes sense for me to keep track of this issue and take some time in the future to make the same improvements here
#372 seems related to this.