Skip to content

Commit 944c5be

Browse files
tobiutobiu
committed
Implement consistent error handling for Memory Core MCP tools #7868 
1 parent 5a04c76 commit 944c5be

5 files changed

Lines changed: 419 additions & 316 deletions

File tree

‎ai/mcp/server/memory-core/services/DatabaseLifecycleService.mjs‎

Lines changed: 64 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -63,47 +63,56 @@ class DatabaseLifecycleService extends Base {
6363
* @returns {Promise<object>} A promise that resolves with the status.
6464
*/
6565
async startDatabase() {
66-
if (this.chromaProcess && !this.chromaProcess.killed) {
67-
return { status: 'already_running', pid: this.chromaProcess.pid, detail: 'Server was started by this process.' };
68-
}
66+
try {
67+
if (this.chromaProcess && !this.chromaProcess.killed) {
68+
return { status: 'already_running', pid: this.chromaProcess.pid, detail: 'Server was started by this process.' };
69+
}
6970

70-
if (await this.isDbRunning()) {
71-
const result = { status: 'already_running', pid: null, detail: 'Server was started externally.' };
72-
this.fire('processActive', { pid: null, managedByService: false, detail: result.detail });
73-
return result;
74-
}
71+
if (await this.isDbRunning()) {
72+
const result = { status: 'already_running', pid: null, detail: 'Server was started externally.' };
73+
this.fire('processActive', { pid: null, managedByService: false, detail: result.detail });
74+
return result;
75+
}
7576

76-
logger.error('Starting ChromaDB (Memory Core) process...');
77+
logger.error('Starting ChromaDB (Memory Core) process...');
7778

78-
await new Promise((resolve, reject) => {
79-
const { port, path: dbPath } = aiConfig.memoryDb;
80-
const args = ['run', '--path', dbPath, '--port', port.toString()];
79+
await new Promise((resolve, reject) => {
80+
const { port, path: dbPath } = aiConfig.memoryDb;
81+
const args = ['run', '--path', dbPath, '--port', port.toString()];
8182

82-
const spawnedProcess = spawn('chroma', args, {
83-
detached: true,
84-
stdio: 'ignore'
85-
});
83+
const spawnedProcess = spawn('chroma', args, {
84+
detached: true,
85+
stdio: 'ignore'
86+
});
8687

87-
spawnedProcess.on('spawn', () => {
88-
this.chromaProcess = spawnedProcess;
89-
logger.log(`ChromaDB (Memory Core) process started with PID: ${this.chromaProcess.pid}`);
90-
resolve();
91-
});
88+
spawnedProcess.on('spawn', () => {
89+
this.chromaProcess = spawnedProcess;
90+
logger.log(`ChromaDB (Memory Core) process started with PID: ${this.chromaProcess.pid}`);
91+
resolve();
92+
});
9293

93-
spawnedProcess.on('error', (err) => {
94-
console.error('Failed to start ChromaDB (Memory Core) process:', err);
95-
this.chromaProcess = null;
96-
reject(err);
97-
});
94+
spawnedProcess.on('error', (err) => {
95+
console.error('Failed to start ChromaDB (Memory Core) process:', err);
96+
this.chromaProcess = null;
97+
reject(err);
98+
});
9899

99-
spawnedProcess.unref();
100-
});
100+
spawnedProcess.unref();
101+
});
101102

102-
await this.waitForHeartbeat();
103+
await this.waitForHeartbeat();
103104

104-
const result = { status: 'started', pid: this.chromaProcess.pid };
105-
this.fire('processActive', { pid: this.chromaProcess.pid, managedByService: true, detail: 'started by service' });
106-
return result;
105+
const result = { status: 'started', pid: this.chromaProcess.pid };
106+
this.fire('processActive', { pid: this.chromaProcess.pid, managedByService: true, detail: 'started by service' });
107+
return result;
108+
} catch (error) {
109+
logger.error('[DatabaseLifecycleService] Error starting database:', error);
110+
return {
111+
error : 'Failed to start database',
112+
message: error.message,
113+
code : 'DATABASE_START_ERROR'
114+
};
115+
}
107116
}
108117

109118
/**
@@ -127,22 +136,31 @@ class DatabaseLifecycleService extends Base {
127136
* @returns {Promise<object>} A promise that resolves with the status.
128137
*/
129138
async stopDatabase() {
130-
if (!this.chromaProcess || this.chromaProcess.killed) {
131-
return { status: 'not_running', detail: 'No process was started by this server.' };
132-
}
139+
try {
140+
if (!this.chromaProcess || this.chromaProcess.killed) {
141+
return { status: 'not_running', detail: 'No process was started by this server.' };
142+
}
133143

134-
return new Promise((resolve) => {
135-
const pid = this.chromaProcess.pid;
136-
this.chromaProcess.on('exit', () => {
137-
logger.log(`ChromaDB process with PID: ${pid} has been stopped.`);
138-
this.chromaProcess = null;
139-
const result = { status: 'stopped' };
140-
this.fire('processStopped', { pid, managedByService: true });
141-
resolve(result);
144+
return new Promise((resolve) => {
145+
const pid = this.chromaProcess.pid;
146+
this.chromaProcess.on('exit', () => {
147+
logger.log(`ChromaDB process with PID: ${pid} has been stopped.`);
148+
this.chromaProcess = null;
149+
const result = { status: 'stopped' };
150+
this.fire('processStopped', { pid, managedByService: true });
151+
resolve(result);
152+
});
153+
154+
process.kill(-this.chromaProcess.pid, 'SIGTERM');
142155
});
143-
144-
process.kill(-this.chromaProcess.pid, 'SIGTERM');
145-
});
156+
} catch (error) {
157+
logger.error('[DatabaseLifecycleService] Error stopping database:', error);
158+
return {
159+
error : 'Failed to stop database',
160+
message: error.message,
161+
code : 'DATABASE_STOP_ERROR'
162+
};
163+
}
146164
}
147165

148166
/**

‎ai/mcp/server/memory-core/services/DatabaseService.mjs‎

Lines changed: 71 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,29 @@ class DatabaseService extends Base {
7373
* @returns {Promise<{message: string}>}
7474
*/
7575
async exportDatabase({ include }) {
76-
logger.log('Starting agent memory export...');
77-
let memoryCount = 0, summaryCount = 0;
76+
try {
77+
logger.log('Starting agent memory export...');
78+
let memoryCount = 0, summaryCount = 0;
7879

79-
if (include.includes('memories')) {
80-
const collection = await ChromaManager.getMemoryCollection();
81-
memoryCount = await this.#exportCollection(collection, aiConfig.memoryDb.backupPath, 'memory-backup');
82-
}
80+
if (include.includes('memories')) {
81+
const collection = await ChromaManager.getMemoryCollection();
82+
memoryCount = await this.#exportCollection(collection, aiConfig.memoryDb.backupPath, 'memory-backup');
83+
}
8384

84-
if (include.includes('summaries')) {
85-
const collection = await ChromaManager.getSummaryCollection();
86-
summaryCount = await this.#exportCollection(collection, aiConfig.sessionDb.backupPath, 'summaries-backup');
87-
}
85+
if (include.includes('summaries')) {
86+
const collection = await ChromaManager.getSummaryCollection();
87+
summaryCount = await this.#exportCollection(collection, aiConfig.sessionDb.backupPath, 'summaries-backup');
88+
}
8889

89-
return { message: `Export complete. Exported ${memoryCount} memories and ${summaryCount} summaries.` };
90+
return { message: `Export complete. Exported ${memoryCount} memories and ${summaryCount} summaries.` };
91+
} catch (error) {
92+
logger.error('[DatabaseService] Error exporting database:', error);
93+
return {
94+
error : 'Failed to export database',
95+
message: error.message,
96+
code : 'DATABASE_EXPORT_ERROR'
97+
};
98+
}
9099
}
91100

92101
/**
@@ -97,57 +106,66 @@ class DatabaseService extends Base {
97106
* @returns {Promise<{imported: number, total: number, mode: string}>}
98107
*/
99108
async importDatabase({ file, mode }) {
100-
const filePath = file; // Assuming file object contains path
101-
logger.log(`Starting agent memory import from: ${filePath}`);
102-
103-
if (!await fs.pathExists(filePath)) {
104-
throw new Error(`Backup file not found at ${filePath}`);
105-
}
109+
try {
110+
const filePath = file; // Assuming file object contains path
111+
logger.log(`Starting agent memory import from: ${filePath}`);
106112

107-
// Determine which collection to import into based on filename
108-
const isMemoryBackup = path.basename(filePath).startsWith('memory-backup');
109-
let collection = isMemoryBackup
110-
? await ChromaManager.getMemoryCollection()
111-
: await ChromaManager.getSummaryCollection();
112-
113-
if (mode === 'replace') {
114-
await ChromaManager.client.deleteCollection({ name: collection.name });
115-
116-
if (isMemoryBackup) {
117-
ChromaManager.memoryCollection = null;
118-
collection = await ChromaManager.getMemoryCollection();
119-
} else {
120-
ChromaManager.summaryCollection = null;
121-
collection = await ChromaManager.getSummaryCollection();
113+
if (!await fs.pathExists(filePath)) {
114+
throw new Error(`Backup file not found at ${filePath}`);
122115
}
123116

124-
logger.log('Replaced mode: existing collection cleared and recreated.');
125-
}
117+
// Determine which collection to import into based on filename
118+
const isMemoryBackup = path.basename(filePath).startsWith('memory-backup');
119+
let collection = isMemoryBackup
120+
? await ChromaManager.getMemoryCollection()
121+
: await ChromaManager.getSummaryCollection();
126122

127-
const fileStream = fs.createReadStream(filePath);
128-
const rl = readline.createInterface({input: fileStream, crlfDelay: Infinity});
129-
const records = [];
123+
if (mode === 'replace') {
124+
await ChromaManager.client.deleteCollection({ name: collection.name });
130125

131-
for await (const line of rl) {
132-
records.push(JSON.parse(line));
133-
}
126+
if (isMemoryBackup) {
127+
ChromaManager.memoryCollection = null;
128+
collection = await ChromaManager.getMemoryCollection();
129+
} else {
130+
ChromaManager.summaryCollection = null;
131+
collection = await ChromaManager.getSummaryCollection();
132+
}
134133

135-
if (records.length === 0) {
136-
return { message: 'No records found in backup file to import.' };
137-
}
134+
logger.log('Replaced mode: existing collection cleared and recreated.');
135+
}
136+
137+
const fileStream = fs.createReadStream(filePath);
138+
const rl = readline.createInterface({input: fileStream, crlfDelay: Infinity});
139+
const records = [];
138140

139-
logger.log(`Importing ${records.length} documents into ${collection.name}...`);
141+
for await (const line of rl) {
142+
records.push(JSON.parse(line));
143+
}
140144

141-
await collection.upsert({
142-
ids : records.map(r => r.id),
143-
embeddings: records.map(r => r.embedding),
144-
metadatas : records.map(r => r.metadata),
145-
documents : records.map(r => r.document)
146-
});
145+
if (records.length === 0) {
146+
return { message: 'No records found in backup file to import.' };
147+
}
147148

148-
const count = await collection.count();
149-
logger.log(`Import complete. Collection "${collection.name}" now contains ${count} documents.`);
150-
return { imported: records.length, total: count, mode };
149+
logger.log(`Importing ${records.length} documents into ${collection.name}...`);
150+
151+
await collection.upsert({
152+
ids : records.map(r => r.id),
153+
embeddings: records.map(r => r.embedding),
154+
metadatas : records.map(r => r.metadata),
155+
documents : records.map(r => r.document)
156+
});
157+
158+
const count = await collection.count();
159+
logger.log(`Import complete. Collection "${collection.name}" now contains ${count} documents.`);
160+
return { imported: records.length, total: count, mode };
161+
} catch (error) {
162+
logger.error('[DatabaseService] Error importing database:', error);
163+
return {
164+
error : 'Failed to import database',
165+
message: error.message,
166+
code : 'DATABASE_IMPORT_ERROR'
167+
};
168+
}
151169
}
152170
}
153171

‎ai/mcp/server/memory-core/services/HealthService.mjs‎

Lines changed: 47 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -304,47 +304,58 @@ class HealthService extends Base {
304304
* @returns {Promise<object>} A health status payload with session information
305305
*/
306306
async healthcheck() {
307-
const now = Date.now();
308-
309-
// Only use cache if the previous result was healthy
310-
// Unhealthy/degraded results are never cached to allow immediate recovery
311-
if (this.#cachedHealth &&
312-
this.#cachedHealth.status === 'healthy' &&
313-
this.#lastCheckTime) {
314-
const age = now - this.#lastCheckTime;
315-
316-
// If the cache is still fresh (< 5 minutes old), return it immediately
317-
if (age < this.#cacheDuration) {
318-
logger.debug(`[HealthService] Using cached health status (age: ${Math.round(age / 1000)}s)`);
319-
return this.#cachedHealth;
307+
try {
308+
const now = Date.now();
309+
310+
// Only use cache if the previous result was healthy
311+
// Unhealthy/degraded results are never cached to allow immediate recovery
312+
if (this.#cachedHealth &&
313+
this.#cachedHealth.status === 'healthy' &&
314+
this.#lastCheckTime) {
315+
const age = now - this.#lastCheckTime;
316+
317+
// If the cache is still fresh (< 5 minutes old), return it immediately
318+
if (age < this.#cacheDuration) {
319+
logger.debug(`[HealthService] Using cached health status (age: ${Math.round(age / 1000)}s)`);
320+
return this.#cachedHealth;
321+
}
320322
}
321-
}
322323

323-
// Cache is stale, was unhealthy, or doesn't exist - perform a fresh check
324-
logger.debug('[HealthService] Performing fresh health check');
325-
const health = await this.#performHealthCheck();
326-
327-
// Detect and log meaningful state transitions
328-
// This helps users understand when their fixes (like starting ChromaDB) succeed
329-
if (this.#previousStatus && this.#previousStatus !== health.status) {
330-
if (this.#previousStatus === 'unhealthy' && health.status === 'healthy') {
331-
logger.info('🎉 [HealthService] System recovered! Memory Core is now fully operational.');
332-
} else if (this.#previousStatus === 'unhealthy' && health.status === 'degraded') {
333-
logger.info('⚠️ [HealthService] System partially recovered. ChromaDB is running but some features unavailable.');
334-
} else if (this.#previousStatus === 'degraded' && health.status === 'healthy') {
335-
logger.info('✅ [HealthService] System fully recovered! All features now operational.');
336-
} else if ((this.#previousStatus === 'healthy' || this.#previousStatus === 'degraded') && health.status === 'unhealthy') {
337-
logger.warn('⚠️ [HealthService] System became unhealthy. Tools may fail until dependencies are resolved.');
324+
// Cache is stale, was unhealthy, or doesn't exist - perform a fresh check
325+
logger.debug('[HealthService] Performing fresh health check');
326+
const health = await this.#performHealthCheck();
327+
328+
// Detect and log meaningful state transitions
329+
// This helps users understand when their fixes (like starting ChromaDB) succeed
330+
if (this.#previousStatus && this.#previousStatus !== health.status) {
331+
if (this.#previousStatus === 'unhealthy' && health.status === 'healthy') {
332+
logger.info('🎉 [HealthService] System recovered! Memory Core is now fully operational.');
333+
} else if (this.#previousStatus === 'unhealthy' && health.status === 'degraded') {
334+
logger.info('⚠️ [HealthService] System partially recovered. ChromaDB is running but some features unavailable.');
335+
} else if (this.#previousStatus === 'degraded' && health.status === 'healthy') {
336+
logger.info('✅ [HealthService] System fully recovered! All features now operational.');
337+
} else if ((this.#previousStatus === 'healthy' || this.#previousStatus === 'degraded') && health.status === 'unhealthy') {
338+
logger.warn('⚠️ [HealthService] System became unhealthy. Tools may fail until dependencies are resolved.');
339+
}
338340
}
339-
}
340341

341-
// Update the cache with this fresh result
342-
// Note: Even unhealthy results are stored, but won't be returned from cache
343-
this.#cachedHealth = health;
344-
this.#lastCheckTime = now;
345-
this.#previousStatus = health.status;
342+
// Update the cache with this fresh result
343+
// Note: Even unhealthy results are stored, but won't be returned from cache
344+
this.#cachedHealth = health;
345+
this.#lastCheckTime = now;
346+
this.#previousStatus = health.status;
346347

347-
return health;
348+
return health;
349+
} catch (error) {
350+
logger.error('[HealthService] Unexpected error during health check:', error);
351+
return {
352+
status : 'unhealthy',
353+
details: [`Unexpected error: ${error.message}`],
354+
error : 'Health check failed unexpectedly',
355+
message: error.message,
356+
code : 'HEALTH_CHECK_ERROR'
357+
};
358+
}
348359
}
349360

350361
/**

0 commit comments

Comments
 (0)