From 17464a3da99682f21e0c09195b9d21dcdd94ae6a Mon Sep 17 00:00:00 2001 From: David Snelling Date: Sat, 11 Oct 2025 09:50:29 -0700 Subject: [PATCH] fix: add comprehensive error logging to GCS and S3 storage adapters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enhance both GCS and S3 storage adapters with detailed logging to diagnose why getNode() returns null without error messages on container restart. Root Cause Identified: - BOTH storage adapters had silent failure pattern in getNode() - Outer catch blocks swallowed ALL errors (network, permission, JSON parse, etc.) - No distinction between "file not found" (expected) vs "actual error" (should throw) - This caused 100% failure rate on HNSW index rebuild after container restart Changes to GCS Storage (gcsStorage.ts): - Add success path logging: load attempt → download → parse → success - Add error logging at TOP of catch block before any conditional checks - Log full error details: type, code, message, HTTP status, complete error object - Log exact GCS path being accessed for debugging path mismatches - Distinguish 404 (return null) from other errors (throw) - Properly handle throttling errors Changes to S3 Storage (s3CompatibleStorage.ts): - Apply same comprehensive error logging as GCS - Add success path logging for each operation step - Add error logging before conditional checks - Handle S3-specific error format (NoSuchKey, $metadata.httpStatusCode) - Distinguish 404/NoSuchKey (return null) from other errors (throw) - Properly handle throttling errors Expected Outcome: - Next test run will reveal exact exception causing getNode() to return null - Same diagnostic capability for both GCS and S3 environments - Proper error handling prevents silent failures in production Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/storage/adapters/gcsStorage.ts | 31 ++++-- src/storage/adapters/s3CompatibleStorage.ts | 108 +++++++++++++------- 2 files changed, 94 insertions(+), 45 deletions(-) diff --git a/src/storage/adapters/gcsStorage.ts b/src/storage/adapters/gcsStorage.ts index af54c375..bbe0261d 100644 --- a/src/storage/adapters/gcsStorage.ts +++ b/src/storage/adapters/gcsStorage.ts @@ -524,14 +524,22 @@ export class GcsStorage extends BaseStorage { const key = this.getNounKey(id) // DIAGNOSTIC LOGGING: Show exact path being accessed - this.logger.trace(`Computed GCS key: ${key}`) + prodLog.info(`[getNode] 🔍 Attempting to load:`) + prodLog.info(`[getNode] UUID: ${id}`) + prodLog.info(`[getNode] Path: ${key}`) + prodLog.info(`[getNode] Bucket: ${this.bucketName}`) // Download from GCS const file = this.bucket!.file(key) + + prodLog.info(`[getNode] 📥 Downloading file...`) const [contents] = await file.download() + prodLog.info(`[getNode] ✅ Download successful: ${contents.length} bytes`) // Parse JSON + prodLog.info(`[getNode] 🔧 Parsing JSON...`) const data = JSON.parse(contents.toString()) + prodLog.info(`[getNode] ✅ JSON parsed successfully, id: ${data.id}`) // Convert serialized connections back to Map> const connections = new Map>() @@ -558,23 +566,32 @@ export class GcsStorage extends BaseStorage { } catch (error: any) { this.releaseBackpressure(false, requestId) + // DIAGNOSTIC LOGGING: Log EVERY error before any conditional checks + const key = this.getNounKey(id) + prodLog.error(`[getNode] ❌ EXCEPTION CAUGHT:`) + prodLog.error(`[getNode] UUID: ${id}`) + prodLog.error(`[getNode] Path: ${key}`) + prodLog.error(`[getNode] Bucket: ${this.bucketName}`) + prodLog.error(`[getNode] Error type: ${error?.constructor?.name || typeof error}`) + prodLog.error(`[getNode] Error code: ${JSON.stringify(error?.code)}`) + prodLog.error(`[getNode] Error message: ${error?.message || String(error)}`) + prodLog.error(`[getNode] Error object:`, JSON.stringify(error, null, 2)) + // Check if this is a "not found" error if (error.code === 404) { - // DIAGNOSTIC LOGGING: Upgrade 404 errors to WARN level with full details - const key = this.getNounKey(id) - prodLog.warn(`[getNode] ❌ 404 NOT FOUND: File does not exist at GCS path: ${key}`) - prodLog.warn(`[getNode] UUID: ${id}`) - prodLog.warn(`[getNode] Bucket: ${this.bucketName}`) - prodLog.warn(`[getNode] This suggests a path mismatch or the file was not written correctly`) + prodLog.warn(`[getNode] Identified as 404 error - returning null`) return null } // Handle throttling if (this.isThrottlingError(error)) { + prodLog.warn(`[getNode] Identified as throttling error - rethrowing`) await this.handleThrottling(error) throw error } + // All other errors should throw, not return null + prodLog.error(`[getNode] Unhandled error - rethrowing`) this.logger.error(`Failed to get node ${id}:`, error) throw BrainyError.fromError(error, `getNoun(${id})`) } diff --git a/src/storage/adapters/s3CompatibleStorage.ts b/src/storage/adapters/s3CompatibleStorage.ts index 661545e6..2a2c66ea 100644 --- a/src/storage/adapters/s3CompatibleStorage.ts +++ b/src/storage/adapters/s3CompatibleStorage.ts @@ -1079,9 +1079,15 @@ export class S3CompatibleStorage extends BaseStorage { // Use getNounKey() to properly handle sharding const key = this.getNounKey(id) - this.logger.trace(`Getting node ${id} from key: ${key}`) + + // DIAGNOSTIC LOGGING: Show exact path being accessed + prodLog.info(`[getNode] 🔍 Attempting to load:`) + prodLog.info(`[getNode] UUID: ${id}`) + prodLog.info(`[getNode] Path: ${key}`) + prodLog.info(`[getNode] Bucket: ${this.bucketName}`) // Try to get the node from the nouns directory + prodLog.info(`[getNode] 📥 Downloading file...`) const response = await this.s3Client!.send( new GetObjectCommand({ Bucket: this.bucketName, @@ -1091,53 +1097,79 @@ export class S3CompatibleStorage extends BaseStorage { // Check if response is null or undefined if (!response || !response.Body) { - this.logger.trace(`No node found for ${id}`) + prodLog.warn(`[getNode] ❌ Response or Body is null/undefined`) return null } // Convert the response body to a string const bodyContents = await response.Body.transformToString() - this.logger.trace(`Retrieved node body for ${id}`) + prodLog.info(`[getNode] ✅ Download successful: ${bodyContents.length} bytes`) // Parse the JSON string - try { - const parsedNode = JSON.parse(bodyContents) - this.logger.trace(`Parsed node data for ${id}`) + prodLog.info(`[getNode] 🔧 Parsing JSON...`) + const parsedNode = JSON.parse(bodyContents) + prodLog.info(`[getNode] ✅ JSON parsed successfully, id: ${parsedNode.id}`) - // Ensure the parsed node has the expected properties - if ( - !parsedNode || - !parsedNode.id || - !parsedNode.vector || - !parsedNode.connections - ) { - this.logger.warn(`Invalid node data for ${id}`) - return null - } - - // Convert serialized connections back to Map> - const connections = new Map>() - for (const [level, nodeIds] of Object.entries(parsedNode.connections)) { - connections.set(Number(level), new Set(nodeIds as string[])) - } - - const node = { - id: parsedNode.id, - vector: parsedNode.vector, - connections, - level: parsedNode.level || 0 - } - - this.logger.trace(`Successfully retrieved node ${id}`) - return node - } catch (parseError) { - this.logger.error(`Failed to parse node data for ${id}:`, parseError) + // Ensure the parsed node has the expected properties + if ( + !parsedNode || + !parsedNode.id || + !parsedNode.vector || + !parsedNode.connections + ) { + prodLog.error(`[getNode] ❌ Invalid node data structure for ${id}`) + prodLog.error(`[getNode] Has id: ${!!parsedNode?.id}`) + prodLog.error(`[getNode] Has vector: ${!!parsedNode?.vector}`) + prodLog.error(`[getNode] Has connections: ${!!parsedNode?.connections}`) return null } - } catch (error) { - // Node not found or other error - this.logger.trace(`Node not found for ${id}`) - return null + + // Convert serialized connections back to Map> + const connections = new Map>() + for (const [level, nodeIds] of Object.entries(parsedNode.connections)) { + connections.set(Number(level), new Set(nodeIds as string[])) + } + + const node = { + id: parsedNode.id, + vector: parsedNode.vector, + connections, + level: parsedNode.level || 0 + } + + this.logger.trace(`Successfully retrieved node ${id}`) + return node + } catch (error: any) { + // DIAGNOSTIC LOGGING: Log EVERY error before any conditional checks + const key = this.getNounKey(id) + prodLog.error(`[getNode] ❌ EXCEPTION CAUGHT:`) + prodLog.error(`[getNode] UUID: ${id}`) + prodLog.error(`[getNode] Path: ${key}`) + prodLog.error(`[getNode] Bucket: ${this.bucketName}`) + prodLog.error(`[getNode] Error type: ${error?.constructor?.name || typeof error}`) + prodLog.error(`[getNode] Error name: ${error?.name}`) + prodLog.error(`[getNode] Error code: ${JSON.stringify(error?.Code || error?.code)}`) + prodLog.error(`[getNode] Error message: ${error?.message || String(error)}`) + prodLog.error(`[getNode] HTTP status: ${error?.$metadata?.httpStatusCode}`) + prodLog.error(`[getNode] Error object:`, JSON.stringify(error, null, 2)) + + // Check if this is a "not found" error (S3 uses "NoSuchKey") + if (error?.name === 'NoSuchKey' || error?.Code === 'NoSuchKey' || error?.$metadata?.httpStatusCode === 404) { + prodLog.warn(`[getNode] Identified as 404/NoSuchKey error - returning null`) + return null + } + + // Handle throttling + if (this.isThrottlingError(error)) { + prodLog.warn(`[getNode] Identified as throttling error - rethrowing`) + await this.handleThrottling(error) + throw error + } + + // All other errors should throw, not return null + prodLog.error(`[getNode] Unhandled error - rethrowing`) + this.logger.error(`Failed to get node ${id}:`, error) + throw BrainyError.fromError(error, `getNoun(${id})`) } }