diff options
author | Joas Schilling <213943+nickvergessen@users.noreply.github.com> | 2020-10-01 16:16:10 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-10-01 16:16:10 +0300 |
commit | 965a8fae67a3877f0496a3be4ef9a8dc94d61e80 (patch) | |
tree | 9c7658da4ca7598680169335d819ffef785c0dad | |
parent | 0284eded590d639f9e00f7a8a5dcd31f696ff35e (diff) | |
parent | a101a20c5062026956ddc4e76f73e506684abf65 (diff) |
Merge pull request #761 from nextcloud/backport/758/stable18v18.0.10RC2v18.0.10RC1v18.0.10
[stable18] Also check responses of 400 errors and so delete unknown devices
-rw-r--r-- | lib/Push.php | 56 | ||||
-rw-r--r-- | tests/Unit/PushTest.php | 11 |
2 files changed, 42 insertions, 25 deletions
diff --git a/lib/Push.php b/lib/Push.php index b2114c8..e389d95 100644 --- a/lib/Push.php +++ b/lib/Push.php @@ -22,6 +22,8 @@ namespace OCA\Notifications; +use GuzzleHttp\Exception\ClientException; +use GuzzleHttp\Exception\ServerException; use OC\Authentication\Exceptions\InvalidTokenException; use OC\Authentication\Token\IProvider; use OC\Security\IdentityProof\Key; @@ -189,40 +191,56 @@ class Push { 'notifications' => $notifications, ], ]); + } catch (ClientException $e) { + // Server responded with 4xx (400 Bad Request mostlikely) + $response = $e->getResponse(); + } catch (ServerException $e) { + // Server responded with 5xx + $response = $e->getResponse(); + $body = $response->getBody(); + $error = \is_string($body) ? $body : ('no reason given (' . $response->getStatusCode() . ')'); + + $this->log->debug('Could not send notification to push server [{url}]: {error}', [ + 'error' => $error, + 'url' => $proxyServer, + 'app' => 'notifications', + ]); + + continue; } catch (\Exception $e) { $this->log->logException($e, [ 'app' => 'notifications', - 'level' => $e->getCode() === Http::STATUS_BAD_REQUEST ? ILogger::INFO : ILogger::WARN, + 'level' => ILogger::ERROR, ]); continue; } $status = $response->getStatusCode(); - if ($status === Http::STATUS_SERVICE_UNAVAILABLE && $this->config->getSystemValue('debug', false)) { - $body = $response->getBody(); - $this->log->debug('Could not send notification to push server [{url}]: {error}',[ - 'error' => \is_string($body) ? $body : 'no reason given', + $body = $response->getBody(); + $bodyData = json_decode($body, true); + + if (is_array($bodyData) && isset($bodyData['unknown'], $bodyData['failed'])) { + if (is_array($bodyData['unknown'])) { + // Proxy returns null when the array is empty + foreach ($bodyData['unknown'] as $unknownDevice) { + $this->deletePushTokenByDeviceIdentifier($unknownDevice); + } + } + } elseif ($status !== Http::STATUS_OK) { + $error = \is_string($body) && $bodyData === null ? $body : 'no reason given'; + $this->log->warning('Could not send notification to push server [{url}]: {error}', [ + 'error' => $error, 'url' => $proxyServer, 'app' => 'notifications', ]); - continue; - } - - $body = $response->getBody(); - $bodyData = json_decode($body, true); - if ($status !== Http::STATUS_OK) { - $this->log->error('Could not send notification to push server [{url}]: {error}',[ - 'error' => \is_string($body) && $bodyData === null ? $body : 'no reason given', + } else { + $error = \is_string($body) && $bodyData === null ? $body : 'no reason given'; + $this->log->info('Push notification sent but response was not parsable, using an outdated push proxy? [{url}]: {error}', [ + 'error' => $error, 'url' => $proxyServer, 'app' => 'notifications', ]); } - - if (is_array($bodyData) && !empty($bodyData['unknown']) && is_array($bodyData['unknown'])) { - foreach ($bodyData['unknown'] as $unknownDevice) { - $this->deletePushTokenByDeviceIdentifier($unknownDevice); - } - } } } diff --git a/tests/Unit/PushTest.php b/tests/Unit/PushTest.php index 190cfb0..14148e9 100644 --- a/tests/Unit/PushTest.php +++ b/tests/Unit/PushTest.php @@ -427,10 +427,9 @@ class PushTest extends TestCase { ], ]); - $this->config->expects($this->exactly(2)) + $this->config->expects($this->exactly(1)) ->method('getSystemValue') ->willReturnMap([ - ['debug', false, $isDebug], ['force_language', false, false], ]); @@ -490,7 +489,7 @@ class PushTest extends TestCase { ->method('logException') ->with($e, [ 'app' => 'notifications', - 'level' => ILogger::WARN, + 'level' => ILogger::ERROR, ]); /** @var IResponse|\PHPUnit_Framework_MockObject_MockObject $response1 */ @@ -511,7 +510,7 @@ class PushTest extends TestCase { ->willReturn($response1); $this->logger->expects($this->at(1)) - ->method('error') + ->method('warning') ->with('Could not send notification to push server [{url}]: {error}', [ 'error' => 'no reason given', 'url' => 'badrequest', @@ -535,8 +534,8 @@ class PushTest extends TestCase { ]) ->willReturn($response2); - $this->logger->expects($isDebug ? $this->at(2) : $this->never()) - ->method('debug') + $this->logger->expects($this->at(2)) + ->method('warning') ->with('Could not send notification to push server [{url}]: {error}', [ 'error' => 'Maintenance', 'url' => 'unavailable', |