Need to return response with failure information for processing response on drupal side.

Case for example:
We send notification to several recipients, some tokens was outdated (because app was removed) - need to determine outdated and remove token from drupal.

CommentFileSizeAuthor
#5 return-response-3013940-2-4.diff1.12 KBr.nabiullin
#5 return-response-3013940-4.patch2.96 KBr.nabiullin
#2 Screenshot 2018-11-15 15.17.15.png27.69 KBr.nabiullin
#2 return-response-3013940-2.patch3.39 KBr.nabiullin

Comments

nabiyllin created an issue. See original summary.

r.nabiullin’s picture

r.nabiullin commented
Status: Active » Needs review
StatusFileSize
new return-response-3013940-2.patch3.39 KB
new Screenshot 2018-11-15 15.17.15.png27.69 KB

Also I changed log process, I divided into 2 situations -> when notification has one recipient and multiple, it will more logically, because we not will see error in situation as on screenshot.

andypost’s picture

andypost
he/him
Primary language Russian
commented 
+++ b/src/Service/FirebaseServiceBase.php
@@ -102,31 +104,44 @@ class FirebaseServiceBase implements FirebaseServiceInterface {
+    catch (\Exception $e) {
+      // Error connecting to Firebase API.
+      $this->messenger()->addError($e->getMessage());
+      $this->logger->error($e->getMessage());

it is a bad idea to use messenger in service - better to move it to calling code

andypost’s picture

andypost
he/him
Primary language Russian
commented

Also that's API break because all consumers of the service expects exception which will not be thrown now

r.nabiullin’s picture

r.nabiullin commented
StatusFileSize
new return-response-3013940-4.patch2.96 KB
new return-response-3013940-2-4.diff1.12 KB

Fixed

leopaccanaro’s picture

leopaccanaro commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.