diff --git a/lib/Push.php b/lib/Push.php index 571059c5d..90ca9e85d 100644 --- a/lib/Push.php +++ b/lib/Push.php @@ -301,13 +301,19 @@ protected function sendNotificationsToProxies(): void { 'notifications' => $notifications, ], ]); + $status = $response->getStatusCode(); + $body = $response->getBody(); + $bodyData = json_decode($body, true); } catch (ClientException $e) { // Server responded with 4xx (400 Bad Request mostlikely) $response = $e->getResponse(); + $status = $response->getStatusCode(); + $body = $response->getBody()->getContents(); + $bodyData = json_decode($body, true); } catch (ServerException $e) { // Server responded with 5xx $response = $e->getResponse(); - $body = $response->getBody(); + $body = $response->getBody()->getContents(); $error = \is_string($body) ? $body : ('no reason given (' . $response->getStatusCode() . ')'); $this->log->debug('Could not send notification to push server [{url}]: {error}', [ @@ -328,10 +334,6 @@ protected function sendNotificationsToProxies(): void { continue; } - $status = $response->getStatusCode(); - $body = $response->getBody(); - $bodyData = json_decode($body, true); - if (is_array($bodyData) && array_key_exists('unknown', $bodyData) && array_key_exists('failed', $bodyData)) { if (is_array($bodyData['unknown'])) { // Proxy returns null when the array is empty @@ -347,7 +349,7 @@ protected function sendNotificationsToProxies(): void { $this->printInfo('Push notification sent successfully'); } } elseif ($status !== Http::STATUS_OK) { - $error = \is_string($body) && $body && $bodyData === null ? $body : 'no reason given'; + $error = $body && $bodyData === null ? $body : 'no reason given'; $this->printInfo('Could not send notification to push server [' . $proxyServer . ']: ' . $error); $this->log->warning('Could not send notification to push server [{url}]: {error}', [ 'error' => $error, @@ -355,7 +357,7 @@ protected function sendNotificationsToProxies(): void { 'app' => 'notifications', ]); } else { - $error = \is_string($body) && $body && $bodyData === null ? $body : 'no reason given'; + $error = $body && $bodyData === null ? $body : 'no reason given'; $this->printInfo('Push notification sent but response was not parsable, using an outdated push proxy? [' . $proxyServer . ']: ' . $error); $this->log->info('Push notification sent but response was not parsable, using an outdated push proxy? [{url}]: {error}', [ 'error' => $error, diff --git a/tests/Unit/PushTest.php b/tests/Unit/PushTest.php index 7f5b6e66d..22eb846b0 100644 --- a/tests/Unit/PushTest.php +++ b/tests/Unit/PushTest.php @@ -45,6 +45,7 @@ use OCP\UserStatus\IManager as IUserStatusManager; use PHPUnit\Framework\MockObject\MockObject; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\StreamInterface; use Psr\Log\LoggerInterface; /** @@ -524,9 +525,14 @@ public function testPushToDeviceSending($isDebug) { $response1->expects($this->once()) ->method('getStatusCode') ->willReturn(Http::STATUS_BAD_REQUEST); + /** @var StreamInterface|MockObject $body1 */ + $body1 = $this->createMock(StreamInterface::class); + $body1->expects($this->once()) + ->method('getContents') + ->willReturn(''); $response1->expects($this->once()) ->method('getBody') - ->willReturn(''); + ->willReturn($body1); $e = $this->createMock(ClientException::class); $e->method('getResponse') ->willReturn($response1); @@ -549,9 +555,14 @@ public function testPushToDeviceSending($isDebug) { /** @var ResponseInterface|MockObject $response1 */ $response2 = $this->createMock(ResponseInterface::class); + /** @var StreamInterface|MockObject $body2 */ + $body2 = $this->createMock(StreamInterface::class); + $body2->expects($this->once()) + ->method('getContents') + ->willReturn('Maintenance'); $response2->expects($this->once()) ->method('getBody') - ->willReturn('Maintenance'); + ->willReturn($body2); $e = $this->createMock(ServerException::class); $e->method('getResponse') ->willReturn($response2); @@ -577,6 +588,8 @@ public function testPushToDeviceSending($isDebug) { $response3->expects($this->once()) ->method('getStatusCode') ->willReturn(Http::STATUS_OK); + /** @var StreamInterface|MockObject $body3 */ + $body3 = $this->createMock(StreamInterface::class); $response3->method('getBody') ->willReturn(''); $client->expects($this->at(3)) @@ -593,14 +606,19 @@ public function testPushToDeviceSending($isDebug) { $response4->expects($this->once()) ->method('getStatusCode') ->willReturn(Http::STATUS_BAD_REQUEST); - $response4->expects($this->once()) - ->method('getBody') + /** @var StreamInterface|MockObject $body4 */ + $body4 = $this->createMock(StreamInterface::class); + $body4->expects($this->once()) + ->method('getContents') ->willReturn(json_encode([ 'failed' => 1, 'unknown' => [ '123456' ] ])); + $response4->expects($this->once()) + ->method('getBody') + ->willReturn($body4); $e = $this->createMock(ClientException::class); $e->method('getResponse') ->willReturn($response4);