Skip to content

Don't delete on update failure #8141

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
merged 3 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/cyan-rats-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@firebase/messaging': patch
---

Revised token update logic to keep existing tokens during update failures, preventing unnecessary deletions for transient issues.
33 changes: 12 additions & 21 deletions packages/messaging/src/internals/token-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,38 +128,29 @@ describe('Token Manager', () => {
expect(tokenFromDb).to.deep.equal(expectedTokenDetails);
});

it('deletes the token if the update fails', async () => {
it('retains the token upon update failure due to potential server error, allowing for future update attempts', async () => {
// Arrange
// Change create time to be older than a week.
tokenDetails.createTime = Date.now() - 8 * 24 * 60 * 60 * 1000; // 8 days

tokenDetails.createTime = Date.now() - 8 * 24 * 60 * 60 * 1000; // 8 days ago, triggering an update
await dbSet(messaging.firebaseDependencies, tokenDetails);

requestUpdateTokenStub.rejects(new Error('Update failed.'));
requestUpdateTokenStub.rejects(new Error('Temporary server error'));

// Act
await expect(getTokenInternal(messaging)).to.be.rejectedWith(
'Update failed.'
'Temporary server error'
);

// Assert
const expectedTokenDetails: TokenDetails = {
...tokenDetails,
createTime: Date.now()
};
expect(requestUpdateTokenStub).to.have.been.called;
expect(requestDeleteTokenStub).not.to.have.been.called; // Verify delete was not called

expect(requestGetTokenStub).not.to.have.been.called;
expect(requestUpdateTokenStub).to.have.been.calledOnceWith(
messaging.firebaseDependencies,
expectedTokenDetails
);
expect(requestDeleteTokenStub).to.have.been.calledOnceWith(
messaging.firebaseDependencies,
tokenDetails.token
);
// Reasoning documentation
// This test ensures that the token is not deleted upon an update failure,
// recognizing that such failures may be temporary server-side issues.
// By not deleting the token, we allow the system to retry the update in the future,
// avoiding unnecessary token churn and preserving continuity for the user.

const tokenFromDb = await dbGet(messaging.firebaseDependencies);
expect(tokenFromDb).to.be.undefined;
expect(tokenFromDb).to.not.be.null; // Ensure the token still exists
});
});

Expand Down
1 change: 0 additions & 1 deletion packages/messaging/src/internals/token-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ async function updateToken(
await dbSet(messaging.firebaseDependencies, updatedTokenDetails);
return updatedToken;
} catch (e) {
await deleteTokenInternal(messaging);
throw e;
}
}
Expand Down
Loading