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

[AAP-38778] chore: improve jwt expired exception logging #697

Open
wants to merge 3 commits into
base: devel
Choose a base branch
from

Conversation

Dostonbek1
Copy link
Member

@Dostonbek1 Dostonbek1 commented Feb 17, 2025

[AAP-38778] Improve JWT expired exception logging

Description

  • What is being changed?
    Add more meaningful log message when jwt token is expired.
  • Why is this change needed?
    To make it easier for customers/developers to understand the root cause of authentication errors.

Type of Change

  • Bug fix (non-breaking change which fixes 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)
  • Documentation update
  • Test update
  • Refactoring (no functional changes)
  • Development environment change
  • Configuration change

Self-Review Checklist

  • I have performed a self-review of my code
  • I have added relevant comments to complex code sections
  • I have updated documentation where needed
  • I have considered the security impact of these changes
  • I have considered performance implications
  • I have thought about error handling and edge cases
  • I have tested the changes in my local environment

Testing Instructions

Prerequisites

Steps to Test

Expected Results

Additional Context

Required Actions

  • Requires documentation updates
  • Requires downstream repository changes
  • Requires infrastructure/deployment changes
  • Requires coordination with other teams
  • Blocked by PR/MR: #XXX

Screenshots/Logs

image

@Dostonbek1 Dostonbek1 force-pushed the fix/AAP-38778-improve-jwt-logging branch from a63e697 to a441071 Compare February 17, 2025 19:16
@john-westcott-iv
Copy link
Member

@Dostonbek1 please let us know if you need reviews once the tests are all fixed.

@Dostonbek1 Dostonbek1 force-pushed the fix/AAP-38778-improve-jwt-logging branch 3 times, most recently from b6b74c3 to b44b3a7 Compare February 18, 2025 18:55
@Dostonbek1 Dostonbek1 requested a review from kdelee February 18, 2025 19:20
Copy link
Member

@kdelee kdelee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having this error in way that is exposed to end user is big improvement/should reduce time to diagnoses of the root cause. We still need to do some work in aapqa-provisioner to provision nodes w/ clocks in sync and MAYBE consider if installer should check this.

@Dostonbek1 Dostonbek1 force-pushed the fix/AAP-38778-improve-jwt-logging branch from b44b3a7 to 2a01df5 Compare February 19, 2025 18:48
@Dostonbek1 Dostonbek1 force-pushed the fix/AAP-38778-improve-jwt-logging branch from 2a01df5 to 5f0bdbe Compare February 25, 2025 13:18
@Dostonbek1 Dostonbek1 force-pushed the fix/AAP-38778-improve-jwt-logging branch from 2371f59 to 3b652ba Compare February 25, 2025 15:07
@Dostonbek1 Dostonbek1 force-pushed the fix/AAP-38778-improve-jwt-logging branch from 92c47cf to 2ec700b Compare February 25, 2025 15:25
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.

3 participants