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

[2.0] Do not return error id if we know we did not send the error #671

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class Client implements ClientInterface
private $middlewareStack;

/**
* @var Event The last event that was captured
* @var Event|null The last event that was captured
*/
private $lastEvent;

Expand Down Expand Up @@ -343,7 +343,11 @@ public function capture(array $payload)
$payload
);

$this->send($event);
if (false === $this->send($event)) {
$this->lastEvent = null;

return null;
}

$this->lastEvent = $event;

Expand All @@ -356,15 +360,15 @@ public function capture(array $payload)
public function send(Event $event)
{
if (!$this->config->shouldCapture($event)) {
return;
return false;
}

// should this event be sampled?
if (mt_rand(1, 100) / 100.0 > $this->config->getSampleRate()) {
return;
return false;
}

$this->transport->send($event);
return $this->transport->send($event);
}

/**
Expand Down
6 changes: 4 additions & 2 deletions src/ClientInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public function captureLastError(array $payload = []);
* Gets the last event that was captured by the client. However, it could
* have been sent or still sit in the queue of pending events.
*
* @return Event
* @return Event|null
*/
public function getLastEvent();

Expand All @@ -127,14 +127,16 @@ public function getLastEventId();
*
* @param array $payload The data of the event being captured
*
* @return string
* @return string|null
*/
public function capture(array $payload);

/**
* Sends the given event to the Sentry server.
*
* @param Event $event The event to send
*
* @return bool|null
*/
public function send(Event $event);

Expand Down
13 changes: 13 additions & 0 deletions tests/ClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,19 @@ public function testGetLastEvent()
$this->assertSame($lastEvent, $client->getLastEvent());
}

public function testGetLastEventEmptyIfNotSent()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename testGetLastEventEmptyIfNotSent to testGetLastEventReturnsNullIfLastEventWasNotSent

{
$client = ClientBuilder::create([
'should_capture' => function () {
return false;
},
])->getClient();

$client->capture(['message' => 'foo']);

$this->assertNull($client->getLastEvent());
}

/**
* @group legacy
*
Expand Down