Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 8
Fix/bad notif token + system push notifs#925
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
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
be27ea7
fix: push notification thing
coodos d847049
fix: push notification token cycling
coodos 83edaa7
fix: system message title
coodos 98b85f0
chore: add debug logs
coodos dd64ce2
fix: remove bad error messges which can mess up token states
coodos File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
16 changes: 14 additions & 2 deletions
16 infrastructure/evault-core/src/controllers/NotificationController.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 2 additions & 0 deletions
2 infrastructure/evault-core/src/core/protocol/graphql-server.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
19 changes: 15 additions & 4 deletions
19 infrastructure/evault-core/src/services/MessageNotificationService.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
146 changes: 123 additions & 23 deletions
146 infrastructure/evault-core/src/services/NotificationService.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import { Repository } from "typeorm"; | ||
| import { Verification } from "../entities/Verification"; | ||
| import { Notification } from "../entities/Notification"; | ||
| import { DeviceToken } from "../entities/DeviceToken"; | ||
| export interface DeviceRegistration { | ||
| eName: string; | ||
| @@ -22,10 +23,24 @@ export interface SendNotificationRequest { | ||
| sharedSecret: string; | ||
| } | ||
| const BAD_TOKEN_ERRORS = [ | ||
| "messaging/registration-token-not-valid", | ||
| "messaging/invalid-registration-token", | ||
| "BadDeviceToken", | ||
| "Unregistered", | ||
| "DeviceTokenNotForTopic", | ||
| ]; | ||
| function isBadTokenError(error: unknown): boolean { | ||
| const msg = error instanceof Error ? error.message : String(error); | ||
| return BAD_TOKEN_ERRORS.some((e) => msg.includes(e)); | ||
| } | ||
| export class NotificationService { | ||
| constructor( | ||
| private verificationRepository: Repository<Verification>, | ||
| private notificationRepository: Repository<Notification> | ||
| private notificationRepository: Repository<Notification>, | ||
| private deviceTokenRepository?: Repository<DeviceToken>, | ||
| ) {} | ||
| async registerDevice(registration: DeviceRegistration): Promise<Verification> { | ||
| @@ -56,10 +71,10 @@ export class NotificationService { | ||
| if (verification) { | ||
| verification.platform = registration.platform; | ||
| if (token) { | ||
| const existing = verification.pushTokens ?? []; | ||
| if (!existing.includes(token)) { | ||
| verification.pushTokens = [...existing, token]; | ||
| } | ||
| // Replace all tokens for this device — the latest token from the | ||
| // OS is the only valid one. Appending caused stale tokens to | ||
| // accumulate and never get cleaned up. | ||
| verification.pushTokens = [token]; | ||
| } | ||
| verification.deviceActive = true; | ||
| verification.updatedAt = new Date(); | ||
| @@ -118,6 +133,7 @@ export class NotificationService { | ||
| // Send actual push notification via notification-trigger service | ||
| const triggerUrl = process.env.NOTIFICATION_TRIGGER_URL || `http://localhost:${process.env.NOTIFICATION_TRIGGER_PORT || 3998}`; | ||
| console.log(`[NOTIF] Using trigger URL: ${triggerUrl}`); | ||
| const pushPayload = { | ||
| title: notification.title, | ||
| body: notification.body, | ||
| @@ -152,8 +168,13 @@ export class NotificationService { | ||
| console.log(`[NOTIF] Sending push to ${allTokens.length} token(s) for eName: ${eName}`); | ||
| const pushResults = await Promise.allSettled( | ||
| allTokens.map(async ({ token, platform }) => { | ||
| // Cycle through tokens sequentially: try each one, remove bad tokens | ||
| // inline, and keep going until at least one succeeds. | ||
| const badTokens: string[] = []; | ||
| let delivered = false; | ||
| for (const { token, platform } of allTokens) { | ||
| try { | ||
| const res = await fetch(`${triggerUrl}/api/send`, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| @@ -165,27 +186,106 @@ export class NotificationService { | ||
| signal: AbortSignal.timeout(10000), | ||
| }); | ||
| const data = await res.json(); | ||
| if (!data.success) { | ||
| throw new Error(data.error || "Push send failed"); | ||
| if (data.success) { | ||
| console.log(`[NOTIF] Push delivered via token ${token.slice(0, 8)}… for ${eName}`); | ||
| delivered = true; | ||
| // Keep sending to remaining tokens — user may have multiple | ||
| // devices (phone + tablet) that should all receive the notif. | ||
| continue; | ||
| } | ||
| return data; | ||
| }) | ||
| ); | ||
| const pushSucceeded = pushResults.filter(r => r.status === "fulfilled").length; | ||
| const pushFailed = pushResults.filter(r => r.status === "rejected").length; | ||
| if (pushFailed > 0) { | ||
| console.log(`[NOTIF] Push results for ${eName}: ${pushSucceeded} sent, ${pushFailed} failed`); | ||
| pushResults.forEach((r, i) => { | ||
| if (r.status === "rejected") { | ||
| console.error(`[NOTIF] Push failed for token index ${i}:`, r.reason); | ||
| // Send returned an explicit failure | ||
| const error = data.error || "Push send failed"; | ||
| console.error( | ||
| `[NOTIF] Push rejected for token ${token.slice(0, 8)}…\n` + | ||
| ` platform : ${platform ?? "auto-detect"}\n` + | ||
| ` error : ${error}`, | ||
| ); | ||
| if (isBadTokenError(error)) { | ||
| badTokens.push(token); | ||
| console.log(`[NOTIF] Bad token ${token.slice(0, 8)}… queued for removal, trying next…`); | ||
| } | ||
| }); | ||
| } catch (err) { | ||
| const msg = err instanceof Error ? err.message : String(err); | ||
| const errObj = err as Record<string, unknown>; | ||
| const rawCause = errObj?.cause; | ||
| const cause = rawCause | ||
| ? rawCause instanceof Error ? rawCause.message : String(rawCause) | ||
| : null; | ||
| console.error( | ||
| `[NOTIF] Push error for token ${token.slice(0, 8)}…\n` + | ||
| ` platform : ${platform ?? "auto-detect"}\n` + | ||
| ` url : ${triggerUrl}/api/send\n` + | ||
| ` error : ${msg}\n` + | ||
| (cause ? ` cause : ${cause}\n` : "") + | ||
| ` full :`, err, | ||
| ); | ||
| if (msg.includes("fetch failed") || msg.includes("ECONNREFUSED")) { | ||
| console.error(`[NOTIF] notification-trigger service appears to be DOWN at ${triggerUrl} — skipping remaining tokens`); | ||
| break; | ||
| } | ||
| if (isBadTokenError(err)) { | ||
| badTokens.push(token); | ||
| console.log(`[NOTIF] Bad token ${token.slice(0, 8)}… queued for removal, trying next…`); | ||
| } | ||
| } | ||
| } | ||
| // Purge bad tokens from both Verification and DeviceToken tables | ||
| if (badTokens.length > 0) { | ||
| console.log(`[NOTIF] Removing ${badTokens.length} bad token(s) for ${eName}`); | ||
| await this.removeBadTokens(eName, badTokens); | ||
| } | ||
| if (delivered) { | ||
| console.log(`[NOTIF] Push delivered for ${eName}`); | ||
| } else { | ||
| console.log(`[NOTIF] Push sent successfully to ${pushSucceeded} token(s) for ${eName}`); | ||
| console.log(`[NOTIF] Push failed for all ${allTokens.length} token(s) for ${eName}`); | ||
| } | ||
| return pushSucceeded > 0 || pushFailed === 0; | ||
| return delivered; | ||
coodos marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| private async removeBadTokens(eName: string, badTokens: string[]): Promise<void> { | ||
| try { | ||
| // Clean Verification table | ||
| const verifications = await this.verificationRepository.find({ | ||
| where: { linkedEName: eName }, | ||
| }); | ||
| for (const v of verifications) { | ||
| const before = v.pushTokens?.length ?? 0; | ||
| v.pushTokens = (v.pushTokens ?? []).filter((t) => !badTokens.includes(t)); | ||
| if (v.pushTokens.length !== before) { | ||
| v.updatedAt = new Date(); | ||
| await this.verificationRepository.save(v); | ||
| } | ||
| } | ||
| // Clean DeviceToken table | ||
| if (this.deviceTokenRepository) { | ||
| const normalized = eName.startsWith("@") ? eName : `@${eName}`; | ||
| const withoutAt = eName.replace(/^@/, ""); | ||
| const rows = await this.deviceTokenRepository | ||
| .createQueryBuilder("dt") | ||
| .where("dt.eName = :e1 OR dt.eName = :e2", { e1: normalized, e2: withoutAt }) | ||
| .getMany(); | ||
| for (const row of rows) { | ||
| const before = row.tokens.length; | ||
| row.tokens = row.tokens.filter((t) => !badTokens.includes(t)); | ||
| if (row.tokens.length !== before) { | ||
| row.updatedAt = new Date(); | ||
| await this.deviceTokenRepository.save(row); | ||
| } | ||
| } | ||
| } | ||
| } catch (err) { | ||
| console.error(`[NOTIF] Failed to remove bad tokens for ${eName}:`, err); | ||
| } | ||
| } | ||
| async getUndeliveredNotifications(eName: string): Promise<Notification[]> { | ||
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.