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

Multiple Push notificatons gateways #34034

Open
DustpaN-Spb opened this issue Nov 22, 2024 · 4 comments
Open

Multiple Push notificatons gateways #34034

DustpaN-Spb opened this issue Nov 22, 2024 · 4 comments
Labels
Tasked Added to the internal issue tracking

Comments

@DustpaN-Spb
Copy link

DustpaN-Spb commented Nov 22, 2024

Administration -> Settings -> Push -> Gateway
If I set multiple push notification gatways rocketchat server send only to first one
(Multiple lines can be used to specify multiple gateways)

Beсause:
https://github.com/RocketChat/Rocket.Chat/blob/develop/apps/meteor/app/push/server/push.ts#L270
in
private async sendNotificationGateway(

in such "for" logics will work only one time and will exit from a "for" loop after first "return this.sendGatewayPush..." and will not try next gateway from a "this.options.gateways" list.

`for (const gateway of this.options.gateways) {
logger.debug('send to token', app.token);

		if ('apn' in app.token && app.token.apn) {
			countApn.push(app._id);
			return this.sendGatewayPush(gateway, 'apn', app.token.apn, { topic: app.appName, ...gatewayNotification });
		}

		if ('gcm' in app.token && app.token.gcm) {
			countGcm.push(app._id);
			return this.sendGatewayPush(gateway, 'gcm', app.token.gcm, gatewayNotification);
		}
	}`
arth-1 added a commit to arth-1/Rocket.Chat that referenced this issue Nov 22, 2024
Fixed sendNotification function to ensure proper asynchronous execution by removing unnecessary return. Improved error handling and logging to ensure reliable push notifications without affecting the flow of the application
@arth-1
Copy link

arth-1 commented Nov 22, 2024

Fixed with slight logic improvement

@reetp
Copy link

reetp commented Dec 9, 2024

I have referred this to the team but not sure what their view will be on this.

Please be patient.

@reetp reetp added the Tasked Added to the internal issue tracking label Dec 9, 2024
@scuciatto
Copy link
Member

Can you please open a Pull Request with the fix you are proposing?
Please, make sure to check our guideline here

@reetp
Copy link

reetp commented Dec 11, 2024

Fixed with slight logic improvement

Make a proper PR as requested please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tasked Added to the internal issue tracking
Projects
None yet
Development

No branches or pull requests

5 participants
@scuciatto @reetp @DustpaN-Spb @arth-1 and others