Skip to content

Conversation

@HardlyDifficult
Copy link
Collaborator

@HardlyDifficult HardlyDifficult commented Jan 15, 2026

Summary

WebSocket connections in canton-node-sdk fetch an OAuth2 token at connection time but have no mechanism to refresh when the token expires during long-running subscriptions. This causes ACCESS_TOKEN_EXPIRED errors after ~1 hour.

This PR adds proactive token refresh support:

  • AuthenticationManager: Track tokenIssuedAt, add getTokenExpiryTime(), getTokenIssuedAt(), getTokenLifetimeMs() methods
  • BaseClient: Add wrapper methods for token timing
  • WebSocketClient: Add calculateTokenRefreshTime() using max(50% lifetime, 2min before expiry), schedule proactive refresh via onTokenRefreshNeeded callback
  • SubscribeToUpdates: Accept onTokenExpiring and onTokenRefreshNeeded callbacks

Token Refresh Timing

The refresh time is calculated as: max(50% of lifetime, 2 minutes before expiry)

Token Lifetime Refresh At Rule Applied
60 seconds 30s 50% rule (protects short tokens)
5 minutes 3min 2-min buffer wins
1 hour 58min 2-min buffer wins

Usage Example

await jsonClient.subscribeToUpdates({
  beginExclusive: currentOffset,
  onTokenExpiring: () => {
    console.log('Token expiring soon, preparing for reconnection...');
  },
  onTokenRefreshNeeded: () => {
    console.log('Token refresh required, will reconnect...');
    return { code: 4000, reason: 'Token refresh' };
  },
  onMessage: (msg) => {
    // Track offset for resumption
    if (msg.update?.Transaction?.value?.offset) {
      lastOffset = msg.update.Transaction.value.offset;
    }
  },
});
// After close, clear token and reconnect from lastOffset
jsonClient.clearToken();

Test plan

  • All unit tests pass
  • Lint and build pass
  • Integration tests with LocalNet (requires manual testing with short-lived tokens)
  • Deploy to devnet and monitor for reduced ACCESS_TOKEN_EXPIRED errors

Note

Introduces proactive token refresh to prevent mid-stream auth expiry in long-running WebSocket subscriptions.

  • core/ws/WebSocketClient: new WebSocketOptions with onTokenExpiring/onTokenRefreshNeeded; schedules refresh via calculateTokenRefreshTime (max 50% lifetime, 2min before expiry); logs schedule/trigger; closes socket with optional code/reason; clears timers on close.
  • core/auth/AuthenticationManager: track tokenIssuedAt/tokenExpiry; set on auth and generator; expose getTokenExpiryTime(), getTokenIssuedAt(), getTokenLifetimeMs(); reset on clearToken().
  • core/BaseClient: expose wrappers for token timing getters used by WebSocketClient.
  • clients/.../subscribe-to-updates.ts: accepts and forwards onTokenExpiring/onTokenRefreshNeeded to WebSocketClient; maintains existing message/error handling.

Written by Cursor Bugbot for commit 34e1c59. This will update automatically on new commits. Configure here.

WebSocket connections fetch an OAuth2 token at connection time but have no
mechanism to refresh when the token expires during long-running subscriptions.
This causes ACCESS_TOKEN_EXPIRED errors after ~1 hour.

Changes:
- AuthenticationManager: Track tokenIssuedAt, add getTokenExpiryTime(),
  getTokenIssuedAt(), getTokenLifetimeMs() methods
- BaseClient: Add wrapper methods for token timing
- WebSocketClient: Add calculateTokenRefreshTime() using max(50% lifetime,
  2min before expiry), schedule proactive refresh via onTokenRefreshNeeded
- SubscribeToUpdates: Accept onTokenExpiring and onTokenRefreshNeeded callbacks

Applications can now receive notification before token expires and handle
reconnection gracefully, resuming from their last processed offset.
@claude
Copy link

claude bot commented Jan 15, 2026

Claude finished @HardlyDifficult's task —— View job


Issues

  • WebSocketClient.ts:48-56 Exported utility function lacks unit tests - calculateTokenRefreshTime implements critical business logic that should be thoroughly tested with edge cases
  • AuthenticationManager.ts:169-174 Missing null safety - getTokenLifetimeMs() should handle case where tokenExpiry is null but tokenIssuedAt is not (though unlikely in practice)
  • WebSocketClient.ts:108-139 Token refresh scheduling has potential race condition - if multiple tokens are issued rapidly, timer could be set up incorrectly

Improvements

  • subscribe-to-updates.ts:196-206 WebSocket options building could be simplified using object spread syntax
  • WebSocketClient.ts:110 Consider making refresh delay calculation more explicit about minimum delay handling
  • AuthenticationManager.ts:42-44 Generator token cache duration (1 minute) seems arbitrary - consider making it configurable

Full Analysis

Overall Assessment

This is a well-designed solution to a real problem. The proactive token refresh mechanism addresses the ACCESS_TOKEN_EXPIRED errors in long-running WebSocket connections with a thoughtful approach that balances early refresh (for short tokens) with reasonable buffer times (for longer tokens).

Code Quality

Strengths:

  • Clear separation of concerns: Token timing logic is isolated in AuthenticationManager, business logic in calculateTokenRefreshTime(), and WebSocket handling in WebSocketClient
  • Comprehensive logging: All token refresh events are properly logged with timestamps and context
  • Good error handling: Timeout cleanup on connection close prevents memory leaks
  • Backward compatibility: New features are entirely opt-in via callback parameters
  • Type safety: Strong TypeScript typing throughout with proper interfaces

Documentation:

  • Excellent JSDoc comments explaining the refresh algorithm with concrete examples
  • Clear parameter documentation in interfaces
  • Good inline comments explaining business logic

Token Refresh Algorithm

The max(50% lifetime, 2min before expiry) approach is sound:

  • Protects short-lived tokens (≤4 minutes) by refreshing at 50% lifetime
  • Provides adequate buffer for longer tokens by refreshing 2 minutes before expiry
  • Prevents edge case where very short tokens might refresh too late

Algorithm verification:

  • ✅ 60s token → 30s refresh (50% rule, appropriate)
  • ✅ 5min token → 3min refresh (2min buffer rule, appropriate)
  • ✅ 1hr token → 58min refresh (2min buffer rule, appropriate)

Security Considerations

  • ✅ Token values properly redacted in logs ('[REDACTED]')
  • ✅ No token exposure in error messages or debug output
  • ✅ Proper cleanup of timers prevents memory leaks
  • ✅ Authentication failures handled gracefully without exposing sensitive data

Performance Considerations

  • ✅ Minimal overhead - only one setTimeout per connection
  • ✅ No polling - event-driven approach
  • ✅ Proper cleanup prevents resource leaks
  • ✅ Non-blocking async operations throughout

Architecture

Flow is well-designed:

  1. AuthenticationManager tracks token lifecycle and timing
  2. BaseClient exposes token timing methods to consumers
  3. WebSocketClient calculates refresh schedule and manages timer
  4. SubscribeToUpdates provides user-friendly callback interface
  5. Application handles reconnection with fresh token

The callback-based approach allows applications to implement their own reconnection strategies while the SDK handles the timing complexity.

Test Coverage Assessment

Missing tests identified:

  • No unit tests for the calculateTokenRefreshTime() function (critical business logic)
  • No tests for token timing methods in AuthenticationManager
  • No tests for WebSocket token refresh flow
  • Integration test mentioned in test plan but not yet implemented

Existing test infrastructure:

  • Comprehensive test suite exists (test/unit/, test/integration/)
  • Test utilities in place (test/utils/testConfig.ts)
  • Integration tests with LocalNet environment available

Implementation Details

WebSocketClient improvements:

  • Timer cleanup is handled correctly in multiple code paths
  • JSON parsing errors properly close connection with appropriate code (1003)
  • Authorization header follows standard Bearer token pattern

Type safety:

  • WebSocketOptions interface properly typed with optional callbacks
  • Update message types correctly union all possible message formats
  • Zod schema validation ensures runtime type safety

Minor Suggestions

  1. Test the utility function: calculateTokenRefreshTime should have comprehensive unit tests
  2. Consider configuration: The 2-minute buffer could be made configurable for different deployment scenarios
  3. Timer precision: For very short tokens, consider sub-second precision in delay calculation
  4. Metrics: Consider adding metrics/telemetry for token refresh frequency and success rates

Integration Considerations

The PR correctly identifies that integration testing requires manual verification with short-lived tokens. The callback-based design makes it easy to test the timing logic separately from the full WebSocket flow.

Conclusion

This is a high-quality implementation that solves the stated problem with a robust, well-designed approach. The code follows best practices, maintains backward compatibility, and provides a clean API for consumers. The main gap is test coverage for the new functionality.


Todo List:

  • Read repository guidelines (CLAUDE.md)
  • Analyze AuthenticationManager changes
  • Analyze BaseClient changes
  • Analyze WebSocketClient changes
  • Analyze subscribe-to-updates changes
  • Review for security concerns
  • Review for performance considerations
  • Assess test coverage
  • Provide comprehensive feedback

@claude
Copy link

claude bot commented Jan 15, 2026

Claude finished @HardlyDifficult's task —— View job


Issues

  • WebSocketClient.ts:48-56: The calculateTokenRefreshTime function should be private/internal to the class rather than exported as a standalone utility. This exposes implementation details and could lead to inconsistent refresh logic if used elsewhere.

  • WebSocketClient.ts:131: Close code 4000 is not in the standard WebSocket close codes range (3000-3999 is for frameworks). Consider using 3001-3999 or providing clear documentation about this non-standard usage.

  • AuthenticationManager.ts:208: The hardcoded 5-minute buffer in isTokenValid() conflicts with the configurable 2-minute buffer in the refresh logic, creating potential confusion and race conditions.


Full Analysis

Architecture & Design ✅

The implementation follows a clean separation of concerns:

  • AuthenticationManager: Handles token lifecycle and expiry tracking
  • BaseClient: Exposes token timing methods to consumers
  • WebSocketClient: Manages connection lifecycle and refresh scheduling
  • SubscribeToUpdates: Provides application-level interface

The token refresh strategy (max(50% lifetime, 2min before expiry)) is well-thought-out and handles both short and long-lived tokens appropriately.

Code Quality ✅

Strengths:

  • Comprehensive JSDoc documentation with clear examples
  • Proper TypeScript types and interfaces
  • Good error handling and logging throughout
  • Clean async/await patterns
  • Proper resource cleanup (timers, event listeners)

Implementation Details:

  • Token timing correctly tracked in AuthenticationManager.ts:98,102
  • Refresh calculation properly implemented in WebSocketClient.ts:48-56
  • Timer cleanup handled in multiple places (WebSocketClient.ts:190-193,201-205)
  • Optional callback pattern allows flexible usage

Security Considerations ✅

  • Bearer tokens properly redacted in logs (WebSocketClient.ts:105)
  • No token values logged or exposed
  • Standard WebSocket authentication via Authorization header
  • Proper input validation using Zod schemas

Performance Considerations ✅

  • Single timer per WebSocket connection (minimal overhead)
  • Efficient refresh timing calculation
  • Proper cleanup prevents memory leaks
  • No unnecessary token refreshes for bounded subscriptions

Error Handling ✅

  • Comprehensive error handling for WebSocket events
  • Proper conversion of Canton error responses to Error objects
  • Race condition protection with settled flag pattern
  • Connection state properly managed

Test Coverage ⚠️

Missing Tests:

  • No unit tests found for calculateTokenRefreshTime function
  • No tests for token refresh timing logic
  • No tests for WebSocketOptions callbacks
  • No tests for new AuthenticationManager methods (getTokenExpiryTime, getTokenIssuedAt, getTokenLifetimeMs)

Existing Coverage:

  • Basic error handling tests exist in test/unit/core/errors.test.ts
  • Integration tests available but don't cover new functionality

Recommendation: Add unit tests for:

// Example test cases needed
describe('calculateTokenRefreshTime', () => {
  it('uses 50% rule for short tokens')
  it('uses 2-minute buffer for long tokens') 
  it('handles edge cases (very short/long tokens)')
})

describe('AuthenticationManager token timing', () => {
  it('tracks tokenIssuedAt and calculates expiry correctly')
  it('clears timing data on clearToken()')
})

Integration Points ✅

  • Backward compatible - all existing functionality preserved
  • Optional callbacks don't break existing consumers
  • Clear upgrade path via new optional parameters
  • Proper handling of token generators vs OAuth2 flows

Usage Example Validation ✅

The provided usage example in the PR description correctly demonstrates:

  • Callback usage for token refresh handling
  • Offset tracking for resumption
  • Proper cleanup with clearToken()
  • Appropriate close codes and reasons

Minor Improvements

  1. WebSocketClient.ts:52: Consider using Math.round instead of Math.floor for more precise timing
  2. subscribe-to-updates.ts:196-206: The conditional WebSocketOptions building could be simplified using object spread with nullish values
  3. AuthenticationManager.ts:43: The 1-minute cache for token generator could be configurable

Copy link
Contributor

Copilot AI left a comment

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 implements proactive token refresh for WebSocket connections to prevent ACCESS_TOKEN_EXPIRED errors during long-running subscriptions. Tokens are refreshed at max(50% of lifetime, 2 minutes before expiry) to balance short-lived token protection with adequate buffer time.

Changes:

  • Added token timing tracking (tokenIssuedAt, tokenExpiryTime, tokenLifetimeMs) to authentication layer
  • Implemented token refresh scheduling in WebSocket client with configurable callbacks
  • Exposed token refresh hooks through subscribe-to-updates API

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
src/core/auth/AuthenticationManager.ts Track tokenIssuedAt timestamp and expose getters for token timing information
src/core/BaseClient.ts Add wrapper methods to expose token timing from AuthenticationManager
src/core/ws/WebSocketClient.ts Implement calculateTokenRefreshTime() function and proactive refresh scheduling with callbacks
src/clients/ledger-json-api/operations/v2/updates/subscribe-to-updates.ts Accept and forward onTokenExpiring/onTokenRefreshNeeded callbacks to WebSocketClient

Comment on lines +48 to +56
export function calculateTokenRefreshTime(tokenIssuedAt: number, tokenExpiresAt: number): number {
const lifetimeMs = tokenExpiresAt - tokenIssuedAt;
const twoMinutesMs = 2 * 60 * 1000;

const halfLifetime = tokenIssuedAt + Math.floor(lifetimeMs / 2);
const twoMinutesBefore = tokenExpiresAt - twoMinutesMs;

return Math.max(halfLifetime, twoMinutesBefore);
}
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

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

The function does not validate that tokenExpiresAt is after tokenIssuedAt. If tokenExpiresAt <= tokenIssuedAt, the calculation will produce a negative or zero lifetime, resulting in incorrect refresh times. Add validation to ensure tokenExpiresAt > tokenIssuedAt and throw an error if not.

Copilot uses AI. Check for mistakes.
Comment on lines +120 to +137
tokenRefreshTimer = setTimeout(() => {
void (async () => {
// Notify that token is expiring
if (options.onTokenExpiring) {
options.onTokenExpiring();
}

await log('token_refresh_triggered', { reason: 'proactive_refresh' });

// Get close params from callback
const closeParams = options.onTokenRefreshNeeded?.();
const closeCode = closeParams?.code ?? 4000;
const closeReason = closeParams?.reason ?? 'Token refresh required';

// Close the socket to trigger reconnection from the caller
socket.close(closeCode, closeReason);
})();
}, delayMs);
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

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

If the onTokenExpiring callback throws an error, the subsequent onTokenRefreshNeeded callback and socket close will not execute, leaving the connection in an inconsistent state. Wrap the onTokenExpiring call in a try-catch block to ensure the refresh flow continues even if this callback fails.

Copilot uses AI. Check for mistakes.
* @param tokenExpiresAt - Timestamp when token expires (ms since epoch)
* @returns Timestamp when refresh should be scheduled (ms since epoch)
*/
export function calculateTokenRefreshTime(tokenIssuedAt: number, tokenExpiresAt: number): number {
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

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

The calculateTokenRefreshTime function implements critical token refresh logic but lacks test coverage. Add unit tests for edge cases: 60-second tokens (50% rule), 5-minute tokens (2-min buffer), 1-hour tokens (2-min buffer), and tokens that have already expired or have negative lifetimes.

Copilot uses AI. Check for mistakes.
Comment on lines +154 to 172
public getTokenExpiryTime(): number | null {
return this.tokenExpiry;
}

/** Returns the timestamp when the current token was issued, in milliseconds since epoch, or null if not available. */
public getTokenIssuedAt(): number | null {
return this.tokenIssuedAt;
}

/**
* Returns the token lifetime in milliseconds, or null if not available. Calculated as the difference between token
* expiry time and issue time.
*/
public getTokenLifetimeMs(): number | null {
if (this.tokenIssuedAt === null || this.tokenExpiry === null) {
return null;
}
return this.tokenExpiry - this.tokenIssuedAt;
}
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

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

The new token timing methods in AuthenticationManager lack test coverage. Other core modules like errors, http-client, logging, and polling have test files. Add tests for the new getters including edge cases where tokens are set via different authentication paths (OAuth2, tokenGenerator, bearerToken).

Copilot uses AI. Check for mistakes.
Comment on lines +196 to +206
let wsOptions: WebSocketOptions | undefined;
if (params.onTokenExpiring !== undefined || params.onTokenRefreshNeeded !== undefined) {
const options: WebSocketOptions = {};
if (params.onTokenExpiring !== undefined) {
options.onTokenExpiring = params.onTokenExpiring;
}
if (params.onTokenRefreshNeeded !== undefined) {
options.onTokenRefreshNeeded = params.onTokenRefreshNeeded;
}
wsOptions = options;
}
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

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

The conditional construction of wsOptions is verbose. Simplify by directly constructing the object with optional properties: const wsOptions = (params.onTokenExpiring || params.onTokenRefreshNeeded) ? { onTokenExpiring: params.onTokenExpiring, onTokenRefreshNeeded: params.onTokenRefreshNeeded } : undefined;. This is cleaner while achieving the same result.

Suggested change
let wsOptions: WebSocketOptions | undefined;
if (params.onTokenExpiring !== undefined || params.onTokenRefreshNeeded !== undefined) {
const options: WebSocketOptions = {};
if (params.onTokenExpiring !== undefined) {
options.onTokenExpiring = params.onTokenExpiring;
}
if (params.onTokenRefreshNeeded !== undefined) {
options.onTokenRefreshNeeded = params.onTokenRefreshNeeded;
}
wsOptions = options;
}
const wsOptions: WebSocketOptions | undefined =
params.onTokenExpiring !== undefined || params.onTokenRefreshNeeded !== undefined
? {
onTokenExpiring: params.onTokenExpiring,
onTokenRefreshNeeded: params.onTokenRefreshNeeded,
}
: undefined;

Copilot uses AI. Check for mistakes.
const refreshTime = calculateTokenRefreshTime(tokenIssuedAt, tokenExpiresAt);
const delayMs = Math.max(0, refreshTime - Date.now());

if (delayMs > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Token refresh skipped when already past refresh time

Medium Severity

When delayMs equals 0 (meaning the calculated refresh time has already passed but the token hasn't expired yet), the condition delayMs > 0 is false and no timer is scheduled. The onTokenExpiring and onTokenRefreshNeeded callbacks are silently skipped, leaving the connection to fail unexpectedly when the token expires. Using >= instead of > would allow setTimeout(fn, 0) to trigger the callbacks immediately on the next event loop tick.

Fix in Cursor Fix in Web


await log('connect', { headers: token ? { Authorization: '[REDACTED]' } : undefined, protocols });

// Schedule proactive token refresh if we have token timing and callbacks
Copy link
Contributor

Choose a reason for hiding this comment

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

Event handlers attached after async yield risks missed events

Medium Severity

The socket is created at line 90, but event handlers aren't attached until line 141. The new await log() calls at lines 105 and 113 yield to the event loop between these points. If the connection completes quickly (e.g., localhost) while logging performs I/O, the open event can fire before its handler is attached, causing it to be missed. The initial request message is never sent, leaving the WebSocket connected but non-functional.

Additional Locations (1)

Fix in Cursor Fix in Web

@HardlyDifficult
Copy link
Collaborator Author

@cursor review all the feedback from other bots in the PR here. Fix where you can make improvements otherwise tell me why they are wrong.

1 similar comment
@HardlyDifficult
Copy link
Collaborator Author

HardlyDifficult commented Jan 15, 2026

@cursor review all the feedback from other bots in the PR here. Fix where you can make improvements otherwise tell me why they are wrong.

@HardlyDifficult HardlyDifficult merged commit 5c14676 into main Jan 15, 2026
13 checks passed
@HardlyDifficult HardlyDifficult deleted the feat/websocket-token-refresh branch January 15, 2026 22:45
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.

2 participants