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

Push notification stopped working in V5 with badge: 1 or Increment #8118

Closed
4 tasks done
ga262 opened this issue Aug 1, 2022 · 9 comments · Fixed by #8162
Closed
4 tasks done

Push notification stopped working in V5 with badge: 1 or Increment #8118

ga262 opened this issue Aug 1, 2022 · 9 comments · Fixed by #8162
Labels
type:bug Impaired feature or lacking behavior that is likely assumed

Comments

@ga262
Copy link

ga262 commented Aug 1, 2022

New Issue Checklist

Issue Description

Push notification with badge stopped working after 5.0.0, it worked up to 4.10.13.

I send push notification with the following code:

const result = await Parse.Push.send({
    where: pushQuery,
    data: data
}, {useMasterKey: true});

with pushQuery the result from the _Installation table and data:

const data = {
    "title": "title",
    "body": "message",
    "type": "type",
    "alert": {"title": "title", "body": "message"},
    "badge": 1,
    "priority": 10,
    "push_type": "alert"
};

If I replace 1 by "Increment" also in the documentation, it also fails.
"badge": "Increment"

The only solution which works is to remove the property badge. In this case, I do receive the notifications. It means it's nor a SDK problem or push parameters, but a problem with this badge parameter.

This JSON DO works:

const data = {
    "title": "title",
    "body": "message",
    "type": "type",
    "alert": {"title": "title", "body": "message"},
    "priority": 10,
    "push_type": "alert"
};

Steps to reproduce

Send a notification with the data properties and valid Installations.

const pushQuery = Installation.getInstallationQueryForUsers(users);  
const data = {
    "title": "title",
    "body": "message",
    "type": "type",
    "alert": {"title": "title", "body": "message"},
    "badge": 1,
    "priority": 10,
    "push_type": "alert"
};
const result = await Parse.Push.send({
    where: pushQuery,
    data: data
}, {useMasterKey: true});

Notifications should be received without "badge" properties, but not received with "badge" parameter set at 1 or "Increment". Is it still supported in 5.0 Parse Server version and above?

Actual Outcome

I do not receive any notification on my iOS and Android apps

The server (both in local and production) gives the following error:

info: beforeSave triggered for _Installation for user undefined:
Input: {"badge":1}
Result: {"object":{"badge":1}} {"className":"_Installation","triggerType":"beforeSave"}

error: _PushStatus AuJRwiG9Nq: error while sending push Performing an update on the path '_id' would modify the immutable field '_id' {"code":66,"index":0,"stack":"MongoServerError: Performing an update on the path '_id' would modify the immutable field '_id'\n    at /home/node_modules/mongodb/lib/operations/update.js:110:33\n    at /home/node_modules/mongodb/lib/cmap/connection_pool.js:273:25\n    at handleOperationResult (/home/node_modules/mongodb/lib/sdam/server.js:363:9)\n    at MessageStream.messageHandler (/home/node_modules/mongodb/lib/cmap/connection.js:474:9)\n    at MessageStream.emit (events.js:375:28)\n    at MessageStream.emit (domain.js:470:12)\n    at processIncomingData (/home/node_modules/mongodb/lib/cmap/message_stream.js:108:16)\n    at MessageStream._write (/home/node_modules/mongodb/lib/cmap/message_stream.js:28:9)\n    at writeOrBuffer (internal/streams/writable.js:358:12)\n    at MessageStream.Writable.write (internal/streams/writable.js:303:10)\n    at TLSSocket.ondata (internal/streams/readable.js:726:22)\n    at TLSSocket.emit (events.js:375:28)\n    at TLSSocket.emit (domain.js:470:12)\n    at addChunk (internal/streams/readable.js:290:12)\n    at readableAddChunk (internal/streams/readable.js:265:9)\n    at TLSSocket.Readable.push (internal/streams/readable.js:204:10)"}

For "badge": "Increment", I have the exact same error but with different input/result:

info: beforeSave triggered for _Installation for user undefined:
Input: {"badge":{"__op":"Increment","amount":1}}
Result: {"object":{"badge":{"__op":"Increment","amount":1}}} {"className":"_Installation","triggerType":"beforeSave"}

Expected Outcome

I expect to receive notifications.

If I remove in the JSON called data the property badge, it works and I DO receive notifications:
In this case, beforeSave for _Installation is not triggered.

The error must come from this _Installation objects update.

Environment

Server

  • Parse Server version: 5.2.4
  • Operating system: Ubuntu
  • Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): Remote
    I also reproduce it on my macOS 12.5 in local, connected to the MondoDB Atlas database

Database

  • System (MongoDB or Postgres): MongoDB
  • Database version: 4.4.15
  • Local or remote host (MongoDB Atlas, mLab, AWS, Azure, Google Cloud, etc): MongoDB Atlas

Client

  • SDK (iOS, Android, JavaScript, PHP, Unity, etc): iOS
  • SDK version: 1.19.3

Logs

@parse-github-assistant
Copy link

parse-github-assistant bot commented Aug 1, 2022

Thanks for opening this issue!

  • 🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.

@ga262 ga262 changed the title Push notification stop working with badge: 1 or Increment Push notification stopped working with badge: 1 or Increment Aug 1, 2022
@ga262 ga262 changed the title Push notification stopped working with badge: 1 or Increment Push notification stopped working in V5 with badge: 1 or Increment Aug 1, 2022
@mtrezza
Copy link
Member

mtrezza commented Aug 1, 2022

The issue may be related to the version of the push adapter. Did you compare the different versions of the adapter in both parse server versions?

@mtrezza mtrezza added the type:bug Impaired feature or lacking behavior that is likely assumed label Aug 1, 2022
@ga262
Copy link
Author

ga262 commented Aug 1, 2022

Thanks for your answer.

When I run npm i for Parse Server 5.4.2, it installs directly the old push-adapter 3.4.1.
When I installed Parse Server 4.10.13, it installed the latest push-adapter 4.1.2.

Even if I add "@parse/push-adapter": "^4.1.2" in the package.json, it installs the push-adapter 4.1.0 for Parse Server 5.4.2. It still have the same error. When I search for "badge" in @parse/push-adapter, it doesn't look related to the push-adapter, but to Parse Server itself.

I'm pretty sure the error comes from this file
https://github.com/parse-community/parse-server/blob/alpha/src/Controllers/PushController.js

    if (body.data && body.data.badge) {
      const badge = body.data.badge;
      let restUpdate = {};
      if (typeof badge == 'string' && badge.toLowerCase() === 'increment') {
        restUpdate = { badge: { __op: 'Increment', amount: 1 } };
      } else if (
        typeof badge == 'object' &&
        typeof badge.__op == 'string' &&
        badge.__op.toLowerCase() == 'increment' &&
        Number(badge.amount)
      ) {
        restUpdate = { badge: { __op: 'Increment', amount: badge.amount } };
      } else if (Number(badge)) {
        restUpdate = { badge: badge };
      } else {
        throw "Invalid value for badge, expected number or 'Increment' or {increment: number}";
      }

      // Force filtering on only valid device tokens
      const updateWhere = applyDeviceTokenExists(where);
      badgeUpdate = () => {
        // Build a real RestQuery so we can use it in RestWrite
        const restQuery = new RestQuery(config, master(config), '_Installation', updateWhere);
        return restQuery.buildRestWhere().then(() => {
          const write = new RestWrite(
            config,
            master(config),
            '_Installation',
            restQuery.restWhere,
            restUpdate
          );
          write.runOptions.many = true;
          return write.execute();
        });
      };
    }

Without the badge parameter, it does not enter the badgeUpdate which makes a Write operation. This Write operation is the source of the log error I think. For some reason, Webstorm does not let me break in this file.

@ga262
Copy link
Author

ga262 commented Aug 1, 2022

I do not really understand how this function works

badgeUpdate = () => {
  // Build a real RestQuery so we can use it in RestWrite
  const restQuery = new _RestQuery.default(config, (0, _Auth.master)(config), '_Installation', updateWhere);
  return restQuery.buildRestWhere().then(() => {
    const write = new _RestWrite.default(config, (0, _Auth.master)(config), '_Installation', restQuery.restWhere, restUpdate);
   write.runOptions.many = true;
    return write.execute();
  });
};

If I comment the writing part (in the parse-server/lib/Controllers/PushController.js), I now receive the notification.

badgeUpdate = () => {
  // Build a real RestQuery so we can use it in RestWrite
  const restQuery = new _RestQuery.default(config, (0, _Auth.master)(config), '_Installation', updateWhere);
  return restQuery.buildRestWhere().then(() => {
  //  const write = new _RestWrite.default(config, (0, _Auth.master)(config), '_Installation', restQuery.restWhere, restUpdate);
  //  write.runOptions.many = true;
    return true;//write.execute();
  });
};

Hence the problem must come from this write function. Maybe I should update my MongoDB from 4.4 to 5.

@mtrezza
Copy link
Member

mtrezza commented Aug 1, 2022

Is the Push controller logic that different between Parse Server 4.x and 5.x? I can't remember there being any major changes.

@ga262
Copy link
Author

ga262 commented Aug 2, 2022

I found the origin of the problem. The Push controller hasn't changed since 2020.

Actually, the change comes from RestWrite.js: https://github.com/parse-community/parse-server/blob/alpha/src/RestWrite.js

When I break in the code, it fails in this.config.database.update(this.className, this.query, this.data, this.runOptions, false, false, this.validSchemaController)

The this.data has changed in RestWrite.prototype.execute in Parse 5.

At the beginning of the execution,

this.data =
{
  "badge": 1
}

When it goes to this.runBeforeSaveTrigger(), Parse 5 includes a property "objectId": undefined, when it was not set in Parse 4.

{
  "badge": 1,
  "objectId": undefined
}

In Parse 4, I got

{
  "badge": 1
}

Then in this.setRequiredFieldsIfNeeded(), it adds the "updatedAt" property but it does not have an impact on the error.

{
  "badge": 1,
  "objectId": undefined,
  "updatedAt": "2022-08-02T16:29:12.573Z"
}

The error given by MongoDB definitely come from the "objectId": undefined
When I force delete this.data.objectId (the only change I did), the push notification is now received. Also, my _Installation objects are updated, which is the expected behaviour.

I've seen that RestWrite had few commits in the last few months and the change must come from one of them. Do you have an easy way to see the changes in RestWrite.prototype.runBeforeSaveTrigger since Parse 4 ?

@mtrezza
Copy link
Member

mtrezza commented Aug 3, 2022

Well investigated! Would you mind opening a PR with a filing test that reproduces this issue?

Do you have an easy way to see the changes in RestWrite.prototype.runBeforeSaveTrigger since Parse 4 ?

Several ways, if you use VSC for example you could simply compare between different commits, see docs. You can also compare between commits directly on GitHub, or just jump to the file and look at the git blame context.

@ga262
Copy link
Author

ga262 commented Aug 7, 2022

Thanks, I spent some time to add the test, but I didn't manage to break in this given test to see if it fails or not, and what the different states are inside this test.

I take a week off, I'll try again in 10 days. I think this is the failing test, but haven't manage to test it yet.

  it('should not throw when trying to create RestWrite with master', done => {
    const data = {badge: 1};
    const install = {deviceToken: "token", foo:"aaa"};
    rest
      .create(config, auth.nobody(config), '_Installation', install)
      .then(() => {
        return rest.find(config, auth.master(config), '_Installation', {});
      })
      .then(query => {
        expect(query[0].get("foo")).equalTo("aaa");
        return new RestWrite(config, auth.master(config), "_Installation", query, data);
      })
      .then(write => {
        expect(write).not.toThrow();
        done();
      });
  });

@mtrezza
Copy link
Member

mtrezza commented Aug 7, 2022

You could just look at the existing tests and rewrite a test that already comes close to what you want to test. Take a look at the /spec folder and you can just add the test to the push related tests. Then submit it as PR and the CI runs the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Impaired feature or lacking behavior that is likely assumed
Projects
None yet
2 participants