search

MatthieuLemoine / electron-push-receiver

A module to bring Web Push support to Electron allowing it to receive notifications from Firebase Cloud Messaging (FCM).
https://medium.com/@MatthieuLemoine/my-journey-to-bring-web-push-support-to-node-and-electron-ce70eea1c0b0
MIT License
191 stars 62 forks source link

Upgrading to 2.0 introduces crashes #18

Closed rahenri closed 5 years ago

rahenri commented 6 years ago

Here is the stack trace:

Uncaught Exception: TypeError: Cannot read property 'value' of undefined at decrypt (/Users/raphael/go/src/github.com/relay/relay/clients/electron/node_modules/push-receiver/src/utils/decrypt/index.js:10:5) at Client._onDataMessage (/Users/raphael/go/src/github.com/relay/relay/clients/electron/node_modules/push-receiver/src/client.js:175:17) at Client._onMessage (/Users/raphael/go/src/github.com/relay/relay/clients/electron/node_modules/push-receiver/src/client.js:164:12) at emitOne (events.js:115:13) at Parser.emit (events.js:210:7) at Parser._onGotMessageBytes (/Users/raphael/go/src/github.com/relay/relay/clients/electron/node_modules/push-receiver/src/parser.js:233:10) at Parser._waitForData (/Users/raphael/go/src/github.com/relay/relay/clients/electron/node_modules/push-receiver/src/parser.js:128:14) at Parser._onGotMessageSize (/Users/raphael/go/src/github.com/relay/relay/clients/electron/node_modules/push-receiver/src/parser.js:191:12) at Parser._onGotMessageTag (/Users/raphael/go/src/github.com/relay/relay/clients/electron/node_modules/push-receiver/src/parser.js:155:10) at Parser._waitForData (/Users/raphael/go/src/github.com/relay/relay/clients/electron/node_modules/push-receiver/src/parser.js:122:14) at Parser._onData (/Users/raphael/go/src/github.com/relay/relay/clients/electron/node_modules/push-receiver/src/parser.js:78:12) at emitOne (events.js:115:13) at TLSSocket.emit (events.js:210:7) at addChunk (_stream_readable.js:252:12) at readableAddChunk (_stream_readable.js:239:11) at TLSSocket.Readable.push (_stream_readable.js:197:10) at TLSWrap.onread (net.js:589:20)

MatthieuLemoine commented 6 years ago

@rahenri Are you able to reproduce it on every notification ?

rahenri commented 6 years ago

I see that on startup everytime, I haven't kept it open any longer than that.

MatthieuLemoine commented 6 years ago

@yurynix raised raised this issue in @ibash's PR in push-receiver. Maybe they have more informations on this

yurynix commented 6 years ago

I've been using GCM, and the messages came in non encrypted, so I just dropped decryption for my use-case.

ibash commented 6 years ago

I wonder what determines whether it's encrypted or not - we've never seen an unencrypted message... Would totally make sense to check for the crypto-key not being there and return it as is.

ibash commented 6 years ago

Also - every once in a while we get push notifications that we can't decrypt (even though they have keys and everything). We've ended up just dropping these because they were a very small percentage (<0.5% IIRC) of notifications (and our client expects to not get all notifications anyway).

MatthieuLemoine commented 6 years ago

@rahenri Do you use GCM or FCM ?

rahenri commented 6 years ago

@MatthieuLemoine I'm using FCM I believe, I'm using Firebase to push those notifications, so that is FCM right?

rahenri commented 6 years ago

Any updates on this? I'm on FCM so messages should be encrypted. Can we just drop the messages that can't be decrypted? Once it crashes, it will continue to crash because the same message will be attempted to be parsed next time until we get a brand new token.

MatthieuLemoine commented 6 years ago

@rahenri Yes I think we can just drop these messages.

MatthieuLemoine commented 6 years ago

Could you setup a PR on push-receiver to fix this ?

rahenri commented 6 years ago

Yeah, I will give it a shot

MatthieuLemoine commented 5 years ago

@rahenri any news ?

MatthieuLemoine commented 5 years ago

Fix released in v2.1.1