Skip to content

fix(auth): remove AuthGuard subscription leak on every route activati…#347

Open
Piyush0000 wants to merge 1 commit into
PSMRI:mainfrom
Piyush0000:fix/amrit-138-authguard-subscription-leak
Open

fix(auth): remove AuthGuard subscription leak on every route activati…#347
Piyush0000 wants to merge 1 commit into
PSMRI:mainfrom
Piyush0000:fix/amrit-138-authguard-subscription-leak

Conversation

@Piyush0000
Copy link
Copy Markdown

AuthGuard.canActivate() subscribed to HttpServiceService.currentLangugae$ on every invocation and stored the emitted value in current_language_set. currentLangugae$ is exposed as BehaviorSubject(...).asObservable() and never completes, so each protected route activation appended a fresh subscriber that was never released. Since AuthGuard is a singleton @Injectable used by canActivate on seven protected routes (service, servicePoint, registrar, nurse-doctor, lab, pharmacist, datasync), repeated navigation accumulated subscribers indefinitely.

current_language_set was assigned inside the subscribe callback but never read anywhere in the guard, in templates, or by reflective access from sibling code. The subscription served no purpose; the correct fix is to delete it rather than convert it to take(1) or takeUntilDestroyed — there is no consumer to feed.

Changes:

  • auth-guard.service.ts: remove the dead current_language_set field, the dead currentLangugae$ subscription, the HttpServiceService dependency injection, and the now-unused import. Guard logic reduces to the session-key validation pipeline it always relied on.

  • auth-guard.service.spec.ts: new spec that locks in the no-leak contract — verifies the guard no longer holds a reference to HttpServiceService or current_language_set, that 20 sequential canActivate calls do not regrow the field, and that the existing navigation semantics (route to /login on invalid session, no-op on valid session) are preserved.

📋 Description

JIRA ID:

Please provide a summary of the change and the motivation behind it. Include relevant context and details.


✅ Type of Change

  • 🐞 Bug fix (non-breaking change which resolves an issue)
  • New feature (non-breaking change which adds functionality)
  • 🔥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 🛠 Refactor (change that is neither a fix nor a new feature)
  • ⚙️ Config change (configuration file or build script updates)
  • 📚 Documentation (updates to docs or readme)
  • 🧪 Tests (adding new or updating existing tests)
  • 🎨 UI/UX (changes that affect the user interface)
  • 🚀 Performance (improves performance)
  • 🧹 Chore (miscellaneous changes that don't modify src or test files)

ℹ️ Additional Information

Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.

…on (PSMRI#138)

AuthGuard.canActivate() subscribed to HttpServiceService.currentLangugae$
on every invocation and stored the emitted value in current_language_set.
currentLangugae$ is exposed as `BehaviorSubject(...).asObservable()` and
never completes, so each protected route activation appended a fresh
subscriber that was never released. Since AuthGuard is a singleton
@Injectable used by canActivate on seven protected routes
(service, servicePoint, registrar, nurse-doctor, lab, pharmacist,
datasync), repeated navigation accumulated subscribers indefinitely.

current_language_set was assigned inside the subscribe callback but
never read anywhere in the guard, in templates, or by reflective
access from sibling code. The subscription served no purpose; the
correct fix is to delete it rather than convert it to take(1) or
takeUntilDestroyed — there is no consumer to feed.

Changes:

* auth-guard.service.ts: remove the dead `current_language_set` field,
  the dead currentLangugae$ subscription, the HttpServiceService
  dependency injection, and the now-unused import. Guard logic
  reduces to the session-key validation pipeline it always relied on.

* auth-guard.service.spec.ts: new spec that locks in the no-leak
  contract — verifies the guard no longer holds a reference to
  HttpServiceService or `current_language_set`, that 20 sequential
  canActivate calls do not regrow the field, and that the existing
  navigation semantics (route to /login on invalid session, no-op on
  valid session) are preserved.
@sonarqubecloud
Copy link
Copy Markdown

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

Warning

Rate limit exceeded

@Piyush0000 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 32 minutes and 20 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d8fa0acd-31a3-4c91-8470-35231e60a7c6

📥 Commits

Reviewing files that changed from the base of the PR and between 91045e5 and 70d90d2.

📒 Files selected for processing (2)
  • src/app/app-modules/core/services/auth-guard.service.spec.ts
  • src/app/app-modules/core/services/auth-guard.service.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant