parse-community / parse-server-push-adapter

A push notification adapter for Parse Server
https://parseplatform.org
MIT License
85 stars 100 forks source link

error while sending push error=PushController: no results in query #103

Closed halavins closed 6 years ago

halavins commented 6 years ago

Issue

Push Controller spams our logs with errors that have no particular reason.

Configuration

Linux Ubuntu 14 LTS Parse-push-adapter version: 2.0.0 Parse Server: 2.6.3 NodeJS: 6.4

How to get such error

Here is the code: ` // Get a pointer to a user, who needs to receive a notification var atPointer = Parse.User.createWithoutData(userID); var pushQuery = new Parse.Query(Parse.Installation);
pushQuery.equalTo("user", atPointer);

// Sending the notification based on PushQuery Parse.Push.send({ where: pushQuery, data: { alert: pushmessage, badge: "Increment" }}); `

Suppose, there are no active Installations for the user with objectId = userID.

PushController ends up with these errors in our logs: 7|parse-wr | error: _PushStatus yAqWZubEGj: error while sending push error=PushController: no results in query 6|parse-wr | error: _PushStatus JcnAoolEuP: error while sending push error=PushController: no results in query 0|parse-wr | error: _PushStatus 0XhRJLH81C: error while sending push error=PushController: no results in query 4|parse-wr | error: _PushStatus pTpmFOrDG4: error while sending push error=PushController: no results in query 6|parse-wr | error: _PushStatus j0cV3dUnfq: error while sending push error=PushController: no results in query 4|parse-wr | error: _PushStatus FVOXCMZKHA: error while sending push error=PushController: no results in query

I wouldn't like to add additional query just to check if a user has any available installations each time someone tries to send a push notification. This check up is done by the PushQuery in the code above itself.

Is it possible to switch off these errors? Are they even valuable? We've got millions of them in the logs since the last update to Parse Server 2.6.3.

flovilmart commented 6 years ago

feel free to open a PR with the silenced logs and we’ll discuss from there.

Closing as it’s not an issue.

halavins commented 6 years ago

@flovilmart, I just think that when we have 0 results in a query it shouldn't be considered as an error.

My logs show only errors, the verbose is set to false. Please consider moving these messages by PushController to Verbose / Debug mode.

halavins commented 6 years ago

Just look at this. Because of this new "Feature", I get this every second (see the attachment):

screenshot 2017-12-07 10 13 57

Now I feel like my servers are consuming CPU just to show the errors, which are not errors at all.

flovilmart commented 6 years ago

That means you’re also spending a lot of CPU creating those _PushStatus Objects for no good reason.

halavins commented 6 years ago

Great thank you for this notice @flovilmart

Is there a way I can check if there are 0 results in query in beforeSave(_PushStatus)? Or how would you suggest optimizing the code below?

// Get a pointer to a user, who needs to receive a notification
var atPointer = Parse.User.createWithoutData(userID);
var pushQuery = new Parse.Query(Parse.Installation);
pushQuery.equalTo("user", atPointer);

// Sending the notification based on PushQuery
// If there are 0 results for the pushQuery, it results in errors by PushController
Parse.Push.send({
where: pushQuery,
data: {
alert: pushmessage,
badge: "Increment"
}});
flovilmart commented 6 years ago

In your case, yes, that’s really unexpected, as also the installation can not have a deviceToken to run off. If you wanna provide a PR, i’ll Be happy to remove it :)

halavins commented 6 years ago

@flovilmart Excuse me, but what did you mean by "in your case, yes"? Could you please give me a kind of recommendation what to do here? How do I check the number of results of a pushQuery in BeforeSave(_PushStatus)?

And what do you mean by "the Installation cannot have a deviceToken to run off"? What's "run off" in this case?

I'm sure I'm not the only one who has such issues. Your advice would be great for everyone who has many users and lots of push notifications sending.

flovilmart commented 6 years ago

I meant that in the way you’re targeting, those logs can be more pollution than useful. I’d be happy to review a PR that would make those logs verbose and not errors