fix: resolve VFS readdir crash due to stale verb count in pagination
Fixes a critical bug where getVerbsWithPagination would crash with
"Cannot read properties of undefined (reading 'replace')" when
the cached totalVerbCount exceeded actual verb files on disk.
Changes:
- Load actual verb files before calculating pagination bounds
- Use actualFileCount instead of stale totalVerbCount cache
- Add null-safety check for undefined array elements
- Track successfullyLoaded count to prevent infinite loops
- Fix getVerbsWithPaginationStreaming to use streaming result for hasMore
This mirrors the fix applied to nouns pagination in commit 5f10f8d
but was never applied to verbs until now.
Reported by Brain Cloud team - VFS directory operations would fail
when metadata entries existed without corresponding verb files.
This commit is contained in:
parent
b77b73e10d
commit
36fdffb27b
1 changed files with 42 additions and 26 deletions
|
|
@ -1115,32 +1115,40 @@ export class FileSystemStorage extends BaseStorage {
|
||||||
const startIndex = options.cursor ? parseInt(options.cursor, 10) : 0
|
const startIndex = options.cursor ? parseInt(options.cursor, 10) : 0
|
||||||
|
|
||||||
try {
|
try {
|
||||||
// Production-scale optimization: Use persisted count for total instead of scanning
|
// Get actual verb files first (critical for accuracy)
|
||||||
const totalCount = this.totalVerbCount || 0
|
const verbFiles = await this.getAllShardedFiles(this.verbsDir)
|
||||||
|
verbFiles.sort() // Consistent ordering for pagination
|
||||||
|
|
||||||
|
// Use actual file count - don't trust cached totalVerbCount
|
||||||
|
// This prevents accessing undefined array elements
|
||||||
|
const actualFileCount = verbFiles.length
|
||||||
|
|
||||||
// For large datasets, warn about performance
|
// For large datasets, warn about performance
|
||||||
if (totalCount > 1000000) {
|
if (actualFileCount > 1000000) {
|
||||||
console.warn(`Very large verb dataset detected (${totalCount} verbs). Performance may be degraded. Consider database storage for optimal performance.`)
|
console.warn(`Very large verb dataset detected (${actualFileCount} verbs). Performance may be degraded. Consider database storage for optimal performance.`)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Calculate pagination bounds
|
|
||||||
const endIndex = Math.min(startIndex + limit, totalCount)
|
|
||||||
const hasMore = endIndex < totalCount
|
|
||||||
|
|
||||||
// For production-scale datasets, use streaming approach
|
// For production-scale datasets, use streaming approach
|
||||||
if (totalCount > 50000) {
|
if (actualFileCount > 50000) {
|
||||||
return await this.getVerbsWithPaginationStreaming(options, startIndex, limit)
|
return await this.getVerbsWithPaginationStreaming(options, startIndex, limit)
|
||||||
}
|
}
|
||||||
|
|
||||||
// For smaller datasets, use the current approach (with optimizations)
|
// Calculate pagination bounds using ACTUAL file count
|
||||||
const verbFiles = await this.getAllShardedFiles(this.verbsDir)
|
const endIndex = Math.min(startIndex + limit, actualFileCount)
|
||||||
verbFiles.sort() // This is still acceptable for <50k files
|
|
||||||
|
|
||||||
// Load the requested page of verbs
|
// Load the requested page of verbs
|
||||||
const verbs: GraphVerb[] = []
|
const verbs: GraphVerb[] = []
|
||||||
|
let successfullyLoaded = 0
|
||||||
|
|
||||||
for (let i = startIndex; i < endIndex; i++) {
|
for (let i = startIndex; i < endIndex; i++) {
|
||||||
const file = verbFiles[i]
|
const file = verbFiles[i]
|
||||||
|
|
||||||
|
// CRITICAL: Null-safety check for undefined array elements
|
||||||
|
if (!file) {
|
||||||
|
console.warn(`Unexpected undefined file at index ${i}, skipping`)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
const id = file.replace('.json', '')
|
const id = file.replace('.json', '')
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
|
@ -1236,14 +1244,19 @@ export class FileSystemStorage extends BaseStorage {
|
||||||
}
|
}
|
||||||
|
|
||||||
verbs.push(verb)
|
verbs.push(verb)
|
||||||
|
successfullyLoaded++
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
console.warn(`Failed to read verb ${id}:`, error)
|
console.warn(`Failed to read verb ${id}:`, error)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// CRITICAL FIX: hasMore based on actual file count, not cached totalVerbCount
|
||||||
|
// Also verify we successfully loaded items (prevents infinite loops on corrupted storage)
|
||||||
|
const hasMore = (endIndex < actualFileCount) && (successfullyLoaded > 0 || startIndex === 0)
|
||||||
|
|
||||||
return {
|
return {
|
||||||
items: verbs,
|
items: verbs,
|
||||||
totalCount,
|
totalCount: actualFileCount, // Return actual count, not cached value
|
||||||
hasMore,
|
hasMore,
|
||||||
nextCursor: hasMore ? String(endIndex) : undefined
|
nextCursor: hasMore ? String(endIndex) : undefined
|
||||||
}
|
}
|
||||||
|
|
@ -1918,7 +1931,6 @@ export class FileSystemStorage extends BaseStorage {
|
||||||
nextCursor?: string
|
nextCursor?: string
|
||||||
}> {
|
}> {
|
||||||
const verbs: GraphVerb[] = []
|
const verbs: GraphVerb[] = []
|
||||||
const totalCount = this.totalVerbCount || 0
|
|
||||||
let processedCount = 0
|
let processedCount = 0
|
||||||
let skippedCount = 0
|
let skippedCount = 0
|
||||||
let resultCount = 0
|
let resultCount = 0
|
||||||
|
|
@ -1927,7 +1939,8 @@ export class FileSystemStorage extends BaseStorage {
|
||||||
|
|
||||||
try {
|
try {
|
||||||
// Stream through sharded directories efficiently
|
// Stream through sharded directories efficiently
|
||||||
const hasMore = await this.streamShardedFiles(
|
// hasMore=false means we reached the end of files, hasMore=true means streaming stopped early
|
||||||
|
const streamingHasMore = await this.streamShardedFiles(
|
||||||
this.verbsDir,
|
this.verbsDir,
|
||||||
depth,
|
depth,
|
||||||
async (filename: string, filePath: string) => {
|
async (filename: string, filePath: string) => {
|
||||||
|
|
@ -1939,7 +1952,7 @@ export class FileSystemStorage extends BaseStorage {
|
||||||
|
|
||||||
// Stop if we have enough results
|
// Stop if we have enough results
|
||||||
if (resultCount >= limit) {
|
if (resultCount >= limit) {
|
||||||
return false // stop streaming
|
return false // stop streaming - more files exist
|
||||||
}
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
|
@ -2011,11 +2024,14 @@ export class FileSystemStorage extends BaseStorage {
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
|
|
||||||
const finalHasMore = (startIndex + resultCount) < totalCount
|
// CRITICAL FIX: Use streaming result for hasMore, not cached totalVerbCount
|
||||||
|
// streamingHasMore=false means we exhausted all files
|
||||||
|
// Also verify we loaded items to prevent infinite loops
|
||||||
|
const finalHasMore = streamingHasMore && (resultCount > 0 || startIndex === 0)
|
||||||
|
|
||||||
return {
|
return {
|
||||||
items: verbs,
|
items: verbs,
|
||||||
totalCount,
|
totalCount: this.totalVerbCount || undefined, // Return cached count as hint only
|
||||||
hasMore: finalHasMore,
|
hasMore: finalHasMore,
|
||||||
nextCursor: finalHasMore ? String(startIndex + resultCount) : undefined
|
nextCursor: finalHasMore ? String(startIndex + resultCount) : undefined
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue