Skip to content
This repository was archived by the owner on Sep 24, 2025. It is now read-only.

fix (packages/nhost-js): if session doesn't have decodedToken treat as expired#49

Merged
dbarrosop merged 3 commits into
mainfrom
refresh-if-not-decodedToken
Aug 14, 2025
Merged

fix (packages/nhost-js): if session doesn't have decodedToken treat as expired#49
dbarrosop merged 3 commits into
mainfrom
refresh-if-not-decodedToken

Conversation

@dbarrosop

@dbarrosop dbarrosop commented Aug 14, 2025

Copy link
Copy Markdown
Member

PR Type

Bug fix


Description

  • Fix session handling for missing decoded tokens

  • Treat sessions without decodedToken as expired

  • Return needsRefresh=true for invalid token sessions

  • Improve session validation logic


Diagram Walkthrough

flowchart LR
  A["Session Check"] --> B["Has decodedToken?"]
  B -- "No" --> C["Mark as Expired & Needs Refresh"]
  B -- "Yes" --> D["Check Token Expiry"]
  C --> E["Return session with refresh flags"]
  D --> F["Normal expiry logic"]
Loading

File Walkthrough

Relevant files
Bug fix
refreshSession.ts
Fix session validation for missing decoded tokens               

packages/nhost-js/src/session/refreshSession.ts

  • Modified _needsRefresh function to handle missing decodedToken
  • Changed return values when decodedToken is invalid
  • Added comment explaining the logic change
  • Now treats sessions without decodedToken as expired
+3/-1     

@dbarrosop dbarrosop changed the title fix (packages/nhost-js): if the session doesn't have decodedToken at it as expired fix (packages/nhost-js): if session doesn't have decodedToken treat as expired Aug 14, 2025
@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Logic Inconsistency

The function returns the session object when decodedToken is invalid but returns null when no session exists. This inconsistency in return types could cause issues for consumers expecting consistent behavior.

return { session: session, needsRefresh: true, sessionExpired: true };
Missing Return

The code block that checks for valid decodedToken now lacks a return statement for the case where the token is valid, which could lead to undefined behavior or execution continuing to subsequent code.

if (!session.decodedToken || !session.decodedToken.exp) {
  // if the session does not have a valid decoded token, treat it as expired
  // as we can't determine its validity
  return { session: session, needsRefresh: true, sessionExpired: true };
}

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Use shorthand property syntax

Remove redundant property name repetition. In TypeScript/JavaScript, when the
property name matches the variable name, you can use shorthand property syntax for
cleaner code.

packages/nhost-js/src/session/refreshSession.ts [154]

-return { session: session, needsRefresh: true, sessionExpired: true };
+return { session, needsRefresh: true, sessionExpired: true };
Suggestion importance[1-10]: 3

__

Why: The suggestion correctly identifies redundant property syntax and proposes valid shorthand notation. However, this is a minor style improvement with minimal impact on functionality.

Low

@dbarrosop dbarrosop requested a review from Copilot August 14, 2025 07:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug in session handling where sessions without a decodedToken were incorrectly treated as valid instead of expired. The fix ensures that sessions missing valid decoded tokens are properly marked as expired and in need of refresh.

  • Changed session validation logic to treat missing decodedToken as expired
  • Modified return values to indicate refresh is needed for invalid token sessions
  • Added explanatory comments for the logic change

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

return { session: null, needsRefresh: false, sessionExpired: false };
// if the session does not have a valid decoded token, treat it as expired
// as we can't determine its validity
return { session: session, needsRefresh: true, sessionExpired: true };

Copilot AI Aug 14, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] The variable name 'session' is redundant in the object property assignment. Consider using shorthand property notation: return { session, needsRefresh: true, sessionExpired: true };

Suggested change
return { session: session, needsRefresh: true, sessionExpired: true };
return { session, needsRefresh: true, sessionExpired: true };

Copilot uses AI. Check for mistakes.
@dbarrosop dbarrosop merged commit 639da49 into main Aug 14, 2025
8 checks passed
@dbarrosop dbarrosop deleted the refresh-if-not-decodedToken branch August 14, 2025 08:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants