Add TokenError, improve exit package utilities, improve error logging

[austin-denoble]

Problem

With the CLI public preview release last week, we've had some reports of errors come through. I noticed we had some deficiencies around both logging / error output (inconsistent output, duplicate log output, etc).

We also were not properly handling the oauth token API responses from auth0. Since these HTTP requests are not going through the SDK and are handled manually, there was a need to add more robust inspection of token response payloads, and properly surface user-friendly error messages on issues with the authentication service. The biggest issue here is the lack of proper error messaging when a user token has expired (24 hours total or 12 hours inactive).

Solution

• Implement new TokenError struct and various utilities for parsing an error from auth responses, and then bubbling that error up through the call stack so it's presented in a reasonable way. I did some testing against what we get back from the auth service on token expiration, and followed RFC6749 The OAuth 2.0 Authorization Framework in terms of the shape of the response and expected error codes from the token endpoints. This may have been a bit overkill, but since the CLI will continue handling it's own client authentication, we should probably start from as robust a place as we can.
• Refactor the exit package a bit. This was mainly for ergonomics, since logging was also tied to calling exit. This was the cause of a lot of logging duplication in spots which I was responsible for implementing. I basically wanted to wrap a zerolog.Event to allow easier chaining at callsites, etc.
• General clean up of logging and usage around exit.Error or exit.Success. For example, we don't generally need to return after calling exit. There were places where errors were being swallowed or not properly bubbled up, etc.
• Add unit tests for /oauth/error.go and exit.go. There's still a lot of unit test coverage missing in general, but trying to add in coverage to new features and things as I go seems prudent.

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)
• This change requires a documentation update
• Infrastructure change (CI configs, etc)
• Non-code change (docs, etc)
• None of the above: (explain here)

Test Plan

For general logging and error cases, test to make sure user-facing messages display as expected. Also validate logging for info, debug, etc using PINECONE_LOG_LEVEL="debug".

Testing TokenError is difficult as we don't have a ton of direct control over the authentication server and its responses. The most consistent testing I was able to do was keeping a stale/expired user token in my cache, and making calls with it. This will attempt a token refresh call, which will return "error": "invalid_grant". This case is consistently testable, so that's what I focused on. The rest of the functionality I hopefully covered through unit testing suites.

	Expired user token error
Before
After

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1211580374844618

[claude]

Claude Code is working…

I'll analyze this and get back to you.

View job run

[claude]

Claude finished @austin-denoble's task —— View job

---

Code Review Complete ✅

I've completed a comprehensive review of this PR focusing on code quality, potential issues, performance, security, and test coverage.

Todo List:

• Read and understand the new TokenError implementation
• Review the exit package improvements
• Analyze error handling changes across the codebase
• Evaluate test coverage
• Check for security concerns
• Assess performance implications
• Provide final review feedback

---

Review Summary

This PR introduces excellent improvements to error handling and user experience for OAuth-related failures. The new TokenError system provides structured, user-friendly error messages while maintaining detailed logging for debugging.

✅ Strengths

1. Excellent Error Handling Design

• TokenError Structure: Well-designed with proper separation between user-facing messages and debug information
• RFC Compliance: Follows OAuth2 RFC6749 error codes correctly
• Comprehensive Coverage: Handles various error scenarios (rate limiting, server issues, token expiration)
• User Experience: Provides helpful, actionable error messages (e.g., "Run pc auth login to sign in again")

2. Strong Test Coverage

• Comprehensive Unit Tests: The error_test.go file provides excellent coverage with 284 lines of tests
• Edge Case Testing: Tests nil cases, different error kinds, and formatting variations
• Test Quality: Well-structured table-driven tests following Go best practices

3. Improved Exit Package

The exit package improvements are minimal but clean - no changes to functionality, just better documentation.

⚠️ Issues Found

1. SECURITY CONCERN: Debug Output Leak

File: internal/pkg/utils/oauth/error.go:198

fmt.Printf("raw: %s\n", raw)

Issue: This debug print statement will output raw OAuth error responses to stdout, potentially exposing sensitive information like tokens, client secrets, or detailed server error messages.

Recommendation: Remove this line or replace with proper logging:

log.Debug().Str("raw_response", raw).Msg("OAuth error response")

2. Memory Safety

File: internal/pkg/utils/oauth/error.go:196

body, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) // 1MB cap

Good: The 1MB limit prevents memory exhaustion attacks
Suggestion: Consider logging when the limit is hit for monitoring purposes

🔧 Code Quality Observations

Positive Patterns

1. Consistent Error Handling: All modified files now use the improved exit.Error() pattern consistently
2. Nil Safety: Proper nil checks in TokenError.Error() and TokenError.Format() methods
3. Custom Formatting: Excellent implementation of fmt.Formatter interface for debugging

Minor Suggestions

1. Error Context: Consider wrapping errors with more context using fmt.Errorf("operation failed: %w", err) pattern where appropriate
2. Constants: Some hardcoded values (like the 2048 truncation limit) could be constants for maintainability

🚀 Performance Assessment

Impact: Minimal performance impact, actually slightly positive:

• Memory: Bounded memory usage with 1MB read limits
• Network: No additional network calls
• CPU: Minimal overhead from error classification logic

📊 Overall Assessment

This is a high-quality PR that significantly improves the user experience for authentication errors. The code is well-structured, thoroughly tested, and follows Go best practices.

Recommendation: Approve with the security fix - Remove the debug print statement on line 198 of error.go, then this PR is ready to merge.

🔄 Required Action

Please remove or replace this line in internal/pkg/utils/oauth/error.go:198:

fmt.Printf("raw: %s\n", raw)  // ← Remove this

[claude]

Claude finished @austin-denoble's task —— View job

---

Code Review Complete ✅

I've completed a comprehensive review of this PR focusing on code quality, potential issues, performance, security, and test coverage.

Todo List:

• Read and understand the new TokenError implementation
• Review the exit package improvements
• Analyze error handling changes across the codebase
• Evaluate test coverage
• Check for security concerns
• Assess performance implications
• Provide final review feedback

---

Review Summary

This PR introduces excellent improvements to error handling and user experience for OAuth-related failures. The new TokenError system provides structured, user-friendly error messages while maintaining detailed logging for debugging.

✅ Strengths

1. Excellent Error Handling Design

• TokenError Structure: Well-designed with proper separation between user-facing messages and debug information
• RFC Compliance: Follows OAuth2 RFC6749 error codes correctly
• Comprehensive Coverage: Handles various error scenarios (rate limiting, server issues, token expiration)
• User Experience: Provides helpful, actionable error messages (e.g., "Run pc auth login to sign in again")

2. Strong Test Coverage

• Comprehensive Unit Tests: The error_test.go file provides excellent coverage with 446 lines of tests
• Edge Case Testing: Tests nil cases, different error kinds, and formatting variations
• Test Quality: Well-structured table-driven tests following Go best practices

3. Improved Exit Package

• Clean Design: The exit package improvements are well-structured with proper separation of concerns
• Testability: Excellent testable design with dependency injection for the exit handler
• Comprehensive Tests: The exit_test.go provides thorough coverage of all exit scenarios

4. Consistent Error Handling Pattern

All modified files now use the improved exit.Error() pattern consistently across the codebase, creating a unified error handling experience.

🔧 Code Quality Observations

Positive Patterns

1. Memory Safety: Proper bounds checking with 1MB limit in NewTokenErrorFromResponse prevents memory exhaustion attacks
2. Nil Safety: Excellent nil checks in TokenError.Error() and TokenError.Format() methods
3. Custom Formatting: Great implementation of fmt.Formatter interface for debugging with %+v support
4. Error Classification: Smart fallback logic using HTTP status codes when OAuth error codes are unknown

Security Assessment

• Fixed Previous Issue: The security concern mentioned in the previous comment about debug output leak (fmt.Printf("raw: %s\n", raw)) has been addressed - this line is no longer present in the code
• Proper Data Handling: Raw response bodies are truncated at 2048 characters to prevent excessive data exposure
• Memory Bounds: 1MB read limit prevents potential DoS attacks

🚀 Performance Assessment

Impact: Minimal performance impact, actually slightly positive:

• Memory: Bounded memory usage with 1MB read limits and 2048-char truncation
• Network: No additional network calls
• CPU: Minimal overhead from error classification logic
• Caching: Token manager properly caches tokens to avoid unnecessary refreshes

📊 Test Coverage Analysis

Excellent Coverage:

• TokenError: 446 lines of comprehensive tests covering all error types, formatting, and edge cases
• Exit Package: 126 lines testing all exit scenarios and convenience functions
• Edge Cases: Tests handle nil inputs, malformed JSON, truncation, and various HTTP status codes

🎯 Code Quality Score: A+

This is exceptionally high-quality code that follows Go best practices:

• Clear separation of concerns
• Comprehensive error handling
• Excellent test coverage
• Proper documentation and comments
• Security-conscious implementation

🏆 Overall Assessment

Recommendation: Approve and Merge

This PR significantly improves the user experience for authentication errors while maintaining robust error logging for debugging. The code is well-structured, thoroughly tested, and follows security best practices.

Key Improvements Delivered:

1. Better UX: Users now get clear, actionable error messages instead of technical OAuth responses
2. Debugging Support: Developers still have access to full error details via %+v formatting
3. Robust Error Classification: Smart mapping of OAuth errors to user-friendly categories
4. Enhanced Exit Utilities: Clean, testable exit handling across the application
5. Security: Proper bounds checking and data truncation to prevent information leakage

No blocking issues found. This PR is ready for merge.

[claude]

Claude finished @austin-denoble's task —— View job

---

Code Review Complete ✅

I've completed a comprehensive review of this PR focusing on code quality, potential bugs, performance considerations, security concerns, and test coverage.

Todo List:

• Read and understand the new TokenError implementation
• Review the exit package improvements
• Analyze error handling changes across the codebase
• Evaluate test coverage
• Check for security concerns
• Assess performance implications
• Provide final review feedback

---

Review Summary

This PR introduces excellent improvements to error handling and user experience for OAuth-related failures. The new TokenError system provides structured, user-friendly error messages while maintaining detailed logging for debugging.

✅ Strengths

1. Excellent Error Handling Design

• TokenError Structure: Well-designed with proper separation between user-facing messages and debug information
• RFC Compliance: Follows OAuth2 RFC6749 error codes correctly
• Comprehensive Coverage: Handles various error scenarios (rate limiting, server issues, token expiration)
• User Experience: Provides helpful, actionable error messages (e.g., "Run pc auth login to sign in again")

2. Strong Test Coverage

• Comprehensive Unit Tests: The error_test.go file provides excellent coverage with 446 lines of tests
• Edge Case Testing: Tests nil cases, different error kinds, and formatting variations
• Test Quality: Well-structured table-driven tests following Go best practices

3. Improved Exit Package

• Clean Design: The exit package improvements are well-structured with proper separation of concerns
• Testability: Excellent testable design with dependency injection for the exit handler
• Comprehensive Tests: The exit_test.go provides thorough coverage of all exit scenarios

4. Consistent Error Handling Pattern

All modified files now use the improved exit.Error() pattern consistently across the codebase, creating a unified error handling experience.

🔧 Code Quality Observations

Positive Patterns

1. Memory Safety: Proper bounds checking with 1MB limit in NewTokenErrorFromResponse prevents memory exhaustion attacks
2. Nil Safety: Excellent nil checks in TokenError.Error() and TokenError.Format() methods
3. Custom Formatting: Great implementation of fmt.Formatter interface for debugging with %+v support
4. Error Classification: Smart fallback logic using HTTP status codes when OAuth error codes are unknown

Security Assessment

• Memory Bounds: 1MB read limit prevents potential DoS attacks
• Data Truncation: Raw response bodies are truncated at 2048 characters to prevent excessive data exposure
• No Sensitive Data Leakage: User-facing error messages don't expose internal details

🚀 Performance Assessment

Impact: Minimal performance impact, actually slightly positive:

• Memory: Bounded memory usage with 1MB read limits and 2048-char truncation
• Network: No additional network calls
• CPU: Minimal overhead from error classification logic
• Caching: Token manager properly caches tokens to avoid unnecessary refreshes

📊 Test Coverage Analysis

Excellent Coverage:

• TokenError: 446 lines of comprehensive tests covering all error types, formatting, and edge cases
• Exit Package: 126 lines testing all exit scenarios and convenience functions
• Edge Cases: Tests handle nil inputs, malformed JSON, truncation, and various HTTP status codes

🎯 Code Quality Score: A+

This is exceptionally high-quality code that follows Go best practices:

• Clear separation of concerns
• Comprehensive error handling
• Excellent test coverage
• Proper documentation and comments
• Security-conscious implementation

🏆 Overall Assessment

Recommendation: Approve and Merge

This PR significantly improves the user experience for authentication errors while maintaining robust error logging for debugging. The code is well-structured, thoroughly tested, and follows security best practices.

Key Improvements Delivered:

1. Better UX: Users now get clear, actionable error messages instead of technical OAuth responses
2. Debugging Support: Developers still have access to full error details via %+v formatting
3. Robust Error Classification: Smart mapping of OAuth errors to user-friendly categories
4. Enhanced Exit Utilities: Clean, testable exit handling across the application
5. Security: Proper bounds checking and data truncation to prevent information leakage

No blocking issues found. This PR is ready for merge.

---