Skip to content

KnockExpoPushNotificaitonProvider assumes all notifications came from knock #589

@creitz

Description

@creitz

Hello!

I'm working on an expo react native application that generates local notifications, as well as receives push notifications from knock via expo. Of course, the local notifications do not come from knock/expo.

It seems that the notification response handler assumes that all notifications come from knock and so the code looks for a knock_message_id.

I think it's fine to check for that field in case the message did come from knock, but it seems the SDK does not correctly handle when knock_message_id is not set, like in the case of local notifications.

The entrypoint for notification interaction is here:

updateKnockMessageStatusFromNotification(notification, "interacted");

..and then in updateKnockMessageStatusFromNotification, updateStatus is called:

return knockClient.messages.updateStatus(messageId, status);

And then updateStatus also doesn't perform any checks for to see if messageId is undefined. Thus, when a user interacts with our locally-generated notifications, it triggers a 404

 (NOBRIDGE) DEBUG  DATADOG: Starting RUM Resource #1749862095019/PUT PUT: https://api.knock.app/v1/messages/undefined/interacted
 (NOBRIDGE) DEBUG  DATADOG: Adding RUM Error “Request failed with status code 404”

This 404 isn't detrimental, but it seems wasteful and is a bit annoying. I can patch the package in the meantime, but is there a way to suppress this or override the handler? I wasn't seeing a way to configure that via props or anything, but it's possible I missed it.

Or perhaps a simple check for undefined message id should be added somewhere in the chain?

Thanks!

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions