diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fa51aa32..eb4cba3c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Fix client creation with proxy (#951) - Allow unsetting the stack trace on an `Event` by calling `Event::setStacktrace(null)` (#961) +- Fix sending of both `event.stacktrace` and `event.exceptions` when `attach_stacktrace = true` (#960) ## 2.3.0 (2020-01-08) diff --git a/src/Client.php b/src/Client.php index 2147258e3..86c07153b 100644 --- a/src/Client.php +++ b/src/Client.php @@ -85,13 +85,7 @@ public function captureMessage(string $message, ?Severity $level = null, ?Scope 'level' => $level, ]; - $event = $this->prepareEvent($payload, $scope, $this->options->shouldAttachStacktrace()); - - if (null === $event) { - return null; - } - - return $this->transport->send($event); + return $this->captureEvent($payload, $scope); } /** @@ -111,7 +105,7 @@ public function captureException(\Throwable $exception, ?Scope $scope = null): ? */ public function captureEvent(array $payload, ?Scope $scope = null): ?string { - $event = $this->prepareEvent($payload, $scope, $this->options->shouldAttachStacktrace()); + $event = $this->prepareEvent($payload, $scope); if (null === $event) { return null; @@ -162,13 +156,12 @@ public function flush(?int $timeout = null): PromiseInterface /** * Assembles an event and prepares it to be sent of to Sentry. * - * @param array $payload the payload that will be converted to an Event - * @param Scope|null $scope optional scope which enriches the Event - * @param bool $withStacktrace True if the event should have and attached stacktrace + * @param array $payload the payload that will be converted to an Event + * @param Scope|null $scope optional scope which enriches the Event * * @return Event|null returns ready to send Event, however depending on options it can be discarded */ - private function prepareEvent(array $payload, ?Scope $scope = null, bool $withStacktrace = false): ?Event + private function prepareEvent(array $payload, ?Scope $scope = null): ?Event { $sampleRate = $this->getOptions()->getSampleRate(); @@ -176,7 +169,7 @@ private function prepareEvent(array $payload, ?Scope $scope = null, bool $withSt return null; } - if ($withStacktrace) { + if ($this->getOptions()->shouldAttachStacktrace() && !isset($payload['exception']) && !isset($payload['stacktrace'])) { $event = $this->eventFactory->createWithStacktrace($payload); } else { $event = $this->eventFactory->create($payload); diff --git a/tests/ClientTest.php b/tests/ClientTest.php index ae4699164..0c5e85bb6 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -140,7 +140,7 @@ public function testCaptureEvent(): void /** * @dataProvider captureEventAttachesStacktraceAccordingToAttachStacktraceOptionDataProvider */ - public function testCaptureEventAttachesStacktraceAccordingToAttachStacktraceOption(bool $shouldAttachStacktrace): void + public function testCaptureEventAttachesStacktraceAccordingToAttachStacktraceOption(bool $attachStacktraceOption, array $payload, bool $shouldAttachStacktrace): void { /** @var TransportInterface&MockObject $transport */ $transport = $this->createMock(TransportInterface::class); @@ -159,19 +159,63 @@ public function testCaptureEventAttachesStacktraceAccordingToAttachStacktraceOpt })) ->willReturn('500a339f3ab2450b96dee542adf36ba7'); - $client = ClientBuilder::create(['attach_stacktrace' => $shouldAttachStacktrace]) + $client = ClientBuilder::create(['attach_stacktrace' => $attachStacktraceOption]) ->setTransportFactory($this->createTransportFactory($transport)) ->getClient(); - $this->assertEquals('500a339f3ab2450b96dee542adf36ba7', $client->captureEvent([])); + $this->assertEquals('500a339f3ab2450b96dee542adf36ba7', $client->captureEvent($payload)); } - public function captureEventAttachesStacktraceAccordingToAttachStacktraceOptionDataProvider(): array + public function captureEventAttachesStacktraceAccordingToAttachStacktraceOptionDataProvider(): \Generator { - return [ - [true], - [false], + yield 'Stacktrace attached when attach_stacktrace = true and both payload.exception and payload.stacktrace are unset' => [ + true, + [], + true, + ]; + + yield 'No stacktrace attached when attach_stacktrace = false' => [ + false, + [], + false, + ]; + + yield 'No stacktrace attached when attach_stacktrace = true and payload.exception is set' => [ + true, + [ + 'exception' => new \Exception(), + ], + false, ]; + + yield 'No stacktrace attached when attach_stacktrace = false and payload.exception is set' => [ + true, + [ + 'exception' => new \Exception(), + ], + false, + ]; + } + + public function testCaptureEventPrefersExplicitStacktrace(): void + { + $explicitStacktrace = $this->createMock(Stacktrace::class); + $payload = ['stacktrace' => $explicitStacktrace]; + + /** @var TransportInterface&MockObject $transport */ + $transport = $this->createMock(TransportInterface::class); + $transport->expects($this->once()) + ->method('send') + ->with($this->callback(static function (Event $event) use ($explicitStacktrace): bool { + return $explicitStacktrace === $event->getStacktrace(); + })) + ->willReturn('500a339f3ab2450b96dee542adf36ba7'); + + $client = ClientBuilder::create(['attach_stacktrace' => true]) + ->setTransportFactory($this->createTransportFactory($transport)) + ->getClient(); + + $this->assertEquals('500a339f3ab2450b96dee542adf36ba7', $client->captureEvent($payload)); } public function testCaptureLastError(): void