Skip to content

regression: do not update logged out sessions#35041

Merged
ggazzo merged 2 commits intodevelopfrom
regression-updated-loggedout-session
Jan 27, 2025
Merged

regression: do not update logged out sessions#35041
ggazzo merged 2 commits intodevelopfrom
regression-updated-loggedout-session

Conversation

@sampaiodiego
Copy link
Member

Proposed changes (including videos or screenshots)

Regression for #35010 which was causing an issue when the same session was logging in and out multiple times.

The problem was that if the new log in was using the same sessionId as a previous one, that was probably logged out already, it was not creating a new session, so when the code was trying to to log out the session it couldn't find it, causing the error to be thrown.

I also changed to not throw an error if the session is not found.

W20250127-13:22:58.343(-3)? (STDERR) === UnHandledPromiseRejection ===
W20250127-13:22:58.344(-3)? (STDERR) Error: Session not found
W20250127-13:22:58.344(-3)? (STDERR)     at app/statistics/server/lib/SAUMonitor.ts:151:11
W20250127-13:22:58.344(-3)? (STDERR)     at processTicksAndRejections (node:internal/process/task_queues:105:5)
W20250127-13:22:58.344(-3)? (STDERR) ---------------------------------
W20250127-13:22:58.344(-3)? (STDERR) Errors like this can cause oplog processing errors.
W20250127-13:22:58.344(-3)? (STDERR) Setting EXIT_UNHANDLEDPROMISEREJECTION will cause the process to exit allowing your service to automatically restart the process
W20250127-13:22:58.344(-3)? (STDERR) Future node.js versions will automatically exit the process
W20250127-13:22:58.344(-3)? (STDERR) =================================

Issue(s)

Steps to test or reproduce

Further comments

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Jan 27, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Jan 27, 2025

⚠️ No Changeset found

Latest commit: 19c839b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@sampaiodiego sampaiodiego added this to the 7.3.0 milestone Jan 27, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jan 27, 2025

PR Preview Action v1.6.0

🚀 View preview at
https://RocketChat.github.io/Rocket.Chat/pr-preview/pr-35041/

Built to branch gh-pages at 2025-01-27 21:39 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@codecov
Copy link

codecov bot commented Jan 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.17%. Comparing base (1ce1f80) to head (19c839b).
Report is 2 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop   #35041   +/-   ##
========================================
  Coverage    59.17%   59.17%           
========================================
  Files         2822     2822           
  Lines        68108    68108           
  Branches     15144    15144           
========================================
  Hits         40303    40303           
  Misses       24974    24974           
  Partials      2831     2831           
Flag Coverage Δ
unit 75.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@ggazzo ggazzo merged commit db58b23 into develop Jan 27, 2025
17 checks passed
@ggazzo ggazzo deleted the regression-updated-loggedout-session branch January 27, 2025 21:44
@ggazzo ggazzo added the stat: QA assured Means it has been tested and approved by a company insider label Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: QA assured Means it has been tested and approved by a company insider

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants