Browse Source

Cloud: add auth logs to the output channel (#4270)

John Richmond 7 months ago
parent
commit
927e210a44

+ 21 - 19
packages/cloud/src/AuthService.ts

@@ -33,15 +33,17 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 	private context: vscode.ExtensionContext
 	private timer: RefreshTimer
 	private state: AuthState = "initializing"
+	private log: (...args: unknown[]) => void
 
 	private credentials: AuthCredentials | null = null
 	private sessionToken: string | null = null
 	private userInfo: CloudUserInfo | null = null
 
-	constructor(context: vscode.ExtensionContext) {
+	constructor(context: vscode.ExtensionContext, log?: (...args: unknown[]) => void) {
 		super()
 
 		this.context = context
+		this.log = log || console.log
 
 		this.timer = new RefreshTimer({
 			callback: async () => {
@@ -72,7 +74,7 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 				}
 			}
 		} catch (error) {
-			console.error("[auth] Error handling credentials change:", error)
+			this.log("[auth] Error handling credentials change:", error)
 		}
 	}
 
@@ -88,7 +90,7 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 
 		this.emit("logged-out", { previousState })
 
-		console.log("[auth] Transitioned to logged-out state")
+		this.log("[auth] Transitioned to logged-out state")
 	}
 
 	private transitionToInactiveSession(credentials: AuthCredentials): void {
@@ -104,7 +106,7 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 
 		this.timer.start()
 
-		console.log("[auth] Transitioned to inactive-session state")
+		this.log("[auth] Transitioned to inactive-session state")
 	}
 
 	/**
@@ -115,7 +117,7 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 	 */
 	public async initialize(): Promise<void> {
 		if (this.state !== "initializing") {
-			console.log("[auth] initialize() called after already initialized")
+			this.log("[auth] initialize() called after already initialized")
 			return
 		}
 
@@ -143,9 +145,9 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 			return authCredentialsSchema.parse(parsedJson)
 		} catch (error) {
 			if (error instanceof z.ZodError) {
-				console.error("[auth] Invalid credentials format:", error.errors)
+				this.log("[auth] Invalid credentials format:", error.errors)
 			} else {
-				console.error("[auth] Failed to parse stored credentials:", error)
+				this.log("[auth] Failed to parse stored credentials:", error)
 			}
 			return null
 		}
@@ -176,7 +178,7 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 			const url = `${getRooCodeApiUrl()}/extension/sign-in?${params.toString()}`
 			await vscode.env.openExternal(vscode.Uri.parse(url))
 		} catch (error) {
-			console.error(`[auth] Error initiating Roo Code Cloud auth: ${error}`)
+			this.log(`[auth] Error initiating Roo Code Cloud auth: ${error}`)
 			throw new Error(`Failed to initiate Roo Code Cloud authentication: ${error}`)
 		}
 	}
@@ -201,7 +203,7 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 			const storedState = this.context.globalState.get(AUTH_STATE_KEY)
 
 			if (state !== storedState) {
-				console.log("[auth] State mismatch in callback")
+				this.log("[auth] State mismatch in callback")
 				throw new Error("Invalid state parameter. Authentication request may have been tampered with.")
 			}
 
@@ -210,9 +212,9 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 			await this.storeCredentials(credentials)
 
 			vscode.window.showInformationMessage("Successfully authenticated with Roo Code Cloud")
-			console.log("[auth] Successfully authenticated with Roo Code Cloud")
+			this.log("[auth] Successfully authenticated with Roo Code Cloud")
 		} catch (error) {
-			console.log(`[auth] Error handling Roo Code Cloud callback: ${error}`)
+			this.log(`[auth] Error handling Roo Code Cloud callback: ${error}`)
 			const previousState = this.state
 			this.state = "logged-out"
 			this.emit("logged-out", { previousState })
@@ -237,14 +239,14 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 				try {
 					await this.clerkLogout(oldCredentials)
 				} catch (error) {
-					console.error("[auth] Error calling clerkLogout:", error)
+					this.log("[auth] Error calling clerkLogout:", error)
 				}
 			}
 
 			vscode.window.showInformationMessage("Logged out from Roo Code Cloud")
-			console.log("[auth] Logged out from Roo Code Cloud")
+			this.log("[auth] Logged out from Roo Code Cloud")
 		} catch (error) {
-			console.log(`[auth] Error logging out from Roo Code Cloud: ${error}`)
+			this.log(`[auth] Error logging out from Roo Code Cloud: ${error}`)
 			throw new Error(`Failed to log out from Roo Code Cloud: ${error}`)
 		}
 	}
@@ -281,7 +283,7 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 	 */
 	private async refreshSession(): Promise<void> {
 		if (!this.credentials) {
-			console.log("[auth] Cannot refresh session: missing credentials")
+			this.log("[auth] Cannot refresh session: missing credentials")
 			this.state = "inactive-session"
 			return
 		}
@@ -292,12 +294,12 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 			this.state = "active-session"
 
 			if (previousState !== "active-session") {
-				console.log("[auth] Transitioned to active-session state")
+				this.log("[auth] Transitioned to active-session state")
 				this.emit("active-session", { previousState })
 				this.fetchUserInfo()
 			}
 		} catch (error) {
-			console.error("[auth] Failed to refresh session", error)
+			this.log("[auth] Failed to refresh session", error)
 			throw error
 		}
 	}
@@ -446,12 +448,12 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 		return this._instance
 	}
 
-	static async createInstance(context: vscode.ExtensionContext) {
+	static async createInstance(context: vscode.ExtensionContext, log?: (...args: unknown[]) => void) {
 		if (this._instance) {
 			throw new Error("AuthService instance already created")
 		}
 
-		this._instance = new AuthService(context)
+		this._instance = new AuthService(context, log)
 		await this._instance.initialize()
 		return this._instance
 	}

+ 5 - 3
packages/cloud/src/CloudService.ts

@@ -18,10 +18,12 @@ export class CloudService {
 	private settingsService: SettingsService | null = null
 	private telemetryClient: TelemetryClient | null = null
 	private isInitialized = false
+	private log: (...args: unknown[]) => void
 
 	private constructor(context: vscode.ExtensionContext, callbacks: CloudServiceCallbacks) {
 		this.context = context
 		this.callbacks = callbacks
+		this.log = callbacks.log || console.log
 		this.authListener = () => {
 			this.callbacks.stateChanged?.()
 		}
@@ -33,7 +35,7 @@ export class CloudService {
 		}
 
 		try {
-			this.authService = await AuthService.createInstance(this.context)
+			this.authService = await AuthService.createInstance(this.context, this.log)
 
 			this.authService.on("inactive-session", this.authListener)
 			this.authService.on("active-session", this.authListener)
@@ -49,12 +51,12 @@ export class CloudService {
 			try {
 				TelemetryService.instance.register(this.telemetryClient)
 			} catch (error) {
-				console.warn("[CloudService] Failed to register TelemetryClient:", error)
+				this.log("[CloudService] Failed to register TelemetryClient:", error)
 			}
 
 			this.isInitialized = true
 		} catch (error) {
-			console.error("[CloudService] Failed to initialize:", error)
+			this.log("[CloudService] Failed to initialize:", error)
 			throw new Error(`Failed to initialize CloudService: ${error}`)
 		}
 	}

+ 1 - 1
packages/cloud/src/__tests__/CloudService.test.ts

@@ -135,7 +135,7 @@ describe("CloudService", () => {
 			const cloudService = await CloudService.createInstance(mockContext, callbacks)
 
 			expect(cloudService).toBeInstanceOf(CloudService)
-			expect(AuthService.createInstance).toHaveBeenCalledWith(mockContext)
+			expect(AuthService.createInstance).toHaveBeenCalledWith(mockContext, expect.any(Function))
 			expect(SettingsService.createInstance).toHaveBeenCalledWith(mockContext, expect.any(Function))
 		})
 

+ 1 - 0
packages/cloud/src/types.ts

@@ -1,3 +1,4 @@
 export interface CloudServiceCallbacks {
 	stateChanged?: () => void
+	log?: (...args: unknown[]) => void
 }

+ 5 - 0
src/extension.ts

@@ -16,6 +16,7 @@ import { CloudService } from "@roo-code/cloud"
 import { TelemetryService, PostHogTelemetryClient } from "@roo-code/telemetry"
 
 import "./utils/path" // Necessary to have access to String.prototype.toPosix.
+import { createOutputChannelLogger, createDualLogger } from "./utils/outputChannelLogger"
 
 import { Package } from "./shared/package"
 import { formatLanguage } from "./shared/language"
@@ -68,9 +69,13 @@ export async function activate(context: vscode.ExtensionContext) {
 		console.warn("Failed to register PostHogTelemetryClient:", error)
 	}
 
+	// Create logger for cloud services
+	const cloudLogger = createDualLogger(createOutputChannelLogger(outputChannel))
+
 	// Initialize Roo Code Cloud service.
 	await CloudService.createInstance(context, {
 		stateChanged: () => ClineProvider.getVisibleInstance()?.postStateToWebview(),
+		log: cloudLogger,
 	})
 
 	// Initialize i18n for internationalization support

+ 86 - 0
src/utils/__tests__/outputChannelLogger.test.ts

@@ -0,0 +1,86 @@
+import * as vscode from "vscode"
+import { createOutputChannelLogger, createDualLogger } from "../outputChannelLogger"
+
+// Mock VSCode output channel
+const mockOutputChannel = {
+	appendLine: jest.fn(),
+} as unknown as vscode.OutputChannel
+
+describe("outputChannelLogger", () => {
+	beforeEach(() => {
+		jest.clearAllMocks()
+		// Clear console.log mock if it exists
+		if (jest.isMockFunction(console.log)) {
+			;(console.log as jest.Mock).mockClear()
+		}
+	})
+
+	describe("createOutputChannelLogger", () => {
+		it("should log strings to output channel", () => {
+			const logger = createOutputChannelLogger(mockOutputChannel)
+			logger("test message")
+
+			expect(mockOutputChannel.appendLine).toHaveBeenCalledWith("test message")
+		})
+
+		it("should log null values", () => {
+			const logger = createOutputChannelLogger(mockOutputChannel)
+			logger(null)
+
+			expect(mockOutputChannel.appendLine).toHaveBeenCalledWith("null")
+		})
+
+		it("should log undefined values", () => {
+			const logger = createOutputChannelLogger(mockOutputChannel)
+			logger(undefined)
+
+			expect(mockOutputChannel.appendLine).toHaveBeenCalledWith("undefined")
+		})
+
+		it("should log Error objects with stack trace", () => {
+			const logger = createOutputChannelLogger(mockOutputChannel)
+			const error = new Error("test error")
+			logger(error)
+
+			expect(mockOutputChannel.appendLine).toHaveBeenCalledWith(expect.stringContaining("Error: test error"))
+		})
+
+		it("should log objects as JSON", () => {
+			const logger = createOutputChannelLogger(mockOutputChannel)
+			const obj = { key: "value", number: 42 }
+			logger(obj)
+
+			expect(mockOutputChannel.appendLine).toHaveBeenCalledWith(JSON.stringify(obj, expect.any(Function), 2))
+		})
+
+		it("should handle multiple arguments", () => {
+			const logger = createOutputChannelLogger(mockOutputChannel)
+			logger("message", 42, { key: "value" })
+
+			expect(mockOutputChannel.appendLine).toHaveBeenCalledTimes(3)
+			expect(mockOutputChannel.appendLine).toHaveBeenNthCalledWith(1, "message")
+			expect(mockOutputChannel.appendLine).toHaveBeenNthCalledWith(2, "42")
+			expect(mockOutputChannel.appendLine).toHaveBeenNthCalledWith(
+				3,
+				JSON.stringify({ key: "value" }, expect.any(Function), 2),
+			)
+		})
+	})
+
+	describe("createDualLogger", () => {
+		it("should log to both output channel and console", () => {
+			const consoleSpy = jest.spyOn(console, "log").mockImplementation()
+			const outputChannelLogger = createOutputChannelLogger(mockOutputChannel)
+			const dualLogger = createDualLogger(outputChannelLogger)
+
+			dualLogger("test message", 42)
+
+			expect(mockOutputChannel.appendLine).toHaveBeenCalledTimes(2)
+			expect(mockOutputChannel.appendLine).toHaveBeenNthCalledWith(1, "test message")
+			expect(mockOutputChannel.appendLine).toHaveBeenNthCalledWith(2, "42")
+			expect(consoleSpy).toHaveBeenCalledWith("test message", 42)
+
+			consoleSpy.mockRestore()
+		})
+	})
+})

+ 51 - 0
src/utils/outputChannelLogger.ts

@@ -0,0 +1,51 @@
+import * as vscode from "vscode"
+
+export type LogFunction = (...args: unknown[]) => void
+
+/**
+ * Creates a logging function that writes to a VSCode output channel
+ * Based on the outputChannelLog implementation from src/extension/api.ts
+ */
+export function createOutputChannelLogger(outputChannel: vscode.OutputChannel): LogFunction {
+	return (...args: unknown[]) => {
+		for (const arg of args) {
+			if (arg === null) {
+				outputChannel.appendLine("null")
+			} else if (arg === undefined) {
+				outputChannel.appendLine("undefined")
+			} else if (typeof arg === "string") {
+				outputChannel.appendLine(arg)
+			} else if (arg instanceof Error) {
+				outputChannel.appendLine(`Error: ${arg.message}\n${arg.stack || ""}`)
+			} else {
+				try {
+					outputChannel.appendLine(
+						JSON.stringify(
+							arg,
+							(key, value) => {
+								if (typeof value === "bigint") return `BigInt(${value})`
+								if (typeof value === "function") return `Function: ${value.name || "anonymous"}`
+								if (typeof value === "symbol") return value.toString()
+								return value
+							},
+							2,
+						),
+					)
+				} catch (error) {
+					outputChannel.appendLine(`[Non-serializable object: ${Object.prototype.toString.call(arg)}]`)
+				}
+			}
+		}
+	}
+}
+
+/**
+ * Creates a logging function that logs to both the output channel and console
+ * Following the pattern from src/extension/api.ts
+ */
+export function createDualLogger(outputChannelLog: LogFunction): LogFunction {
+	return (...args: unknown[]) => {
+		outputChannelLog(...args)
+		console.log(...args)
+	}
+}