Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions src/apis/keyvalue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,4 +224,50 @@ export default class KeyValueAPI implements KeyValueStorage {
span.end();
}
}

/**
* get all keys from the key value storage
*
* @param name - the name of the key value storage
* @returns an array of all keys in the storage
*/
async all(name: string): Promise<string[]> {
const tracer = getTracer();
const currentContext = context.active();

// Create a child span using the current context
const span = tracer.startSpan('agentuity.keyvalue.all', {}, currentContext);

try {
span.setAttribute('name', name);

// Create a new context with the child span
const spanContext = trace.setSpan(currentContext, span);

// Execute the operation within the new context
return await context.with(spanContext, async () => {
const resp = await GET(
`/kv/2025-03-17/keys/${encodeURIComponent(name)}`,
false,
undefined,
undefined,
undefined,
'keyvalue'
);
if (resp.status === 200) {
const keys = await resp.response.json();
span.setStatus({ code: SpanStatusCode.OK });
return keys as string[];
}
throw new Error(
`error getting all keys: ${resp.response.statusText} (${resp.response.status})`
);
Comment on lines +257 to +264
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Handle 404 for non-existent namespaces by returning empty array.

According to the PR objectives, "non-existent/empty namespace behavior (should return [])" is a key design decision. The current implementation throws an error for all non-200 responses, including 404 (non-existent namespace), which conflicts with this requirement.

The get() method (lines 51-55) provides a precedent for handling 404 specially. The all() method should follow the same pattern.

Apply this diff to handle 404 appropriately:

 			return await context.with(spanContext, async () => {
 				const resp = await GET(
 					`/kv/2025-03-17/keys/${encodeURIComponent(name)}`,
 					false,
 					undefined,
 					undefined,
 					undefined,
 					'keyvalue'
 				);
+				if (resp.status === 404) {
+					span.addEvent('namespace-not-found');
+					span.setStatus({ code: SpanStatusCode.OK });
+					return [];
+				}
 				if (resp.status === 200) {
 					const keys = await resp.response.json();
 					span.setStatus({ code: SpanStatusCode.OK });
 					return keys as string[];
 				}
 				throw new Error(
 					`error getting all keys: ${resp.response.statusText} (${resp.response.status})`
 				);
 			});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (resp.status === 200) {
const keys = await resp.response.json();
span.setStatus({ code: SpanStatusCode.OK });
return keys as string[];
}
throw new Error(
`error getting all keys: ${resp.response.statusText} (${resp.response.status})`
);
if (resp.status === 404) {
span.addEvent('namespace-not-found');
span.setStatus({ code: SpanStatusCode.OK });
return [];
}
if (resp.status === 200) {
const keys = await resp.response.json();
span.setStatus({ code: SpanStatusCode.OK });
return keys as string[];
}
throw new Error(
`error getting all keys: ${resp.response.statusText} (${resp.response.status})`
);
🤖 Prompt for AI Agents
In src/apis/keyvalue.ts around lines 257 to 264, change the error handling for
non-200 responses so that a 404 (non-existent namespace) returns an empty array
instead of throwing; specifically, check if resp.status === 404 then set
span.setStatus({ code: SpanStatusCode.OK }) and return [] (mirroring the get()
method pattern), otherwise keep throwing the existing Error with the
statusText/status; ensure any response body is not assumed present for 404 and
that tracing/span status is updated for the successful-but-empty case.

});
} catch (ex) {
recordException(span, ex);
throw ex;
} finally {
span.end();
}
}
}
8 changes: 8 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,14 @@ export interface KeyValueStorage {
* @param key - the key to delete
*/
delete(name: string, key: string): Promise<void>;

/**
* get all keys from the key value storage
*
* @param name - the name of the key value storage
* @returns an array of all keys in the storage
*/
all(name: string): Promise<string[]>;
}

/**
Expand Down
68 changes: 68 additions & 0 deletions test/apis/keyvalue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,4 +141,72 @@ describe('KeyValueAPI', () => {
await expect(keyValueAPI.get('test-store', 'test-key')).rejects.toThrow();
});
});

describe('all', () => {
it('should retrieve all keys successfully', async () => {
const mockResponse = {
status: 200,
response: {
json: () => Promise.resolve(['key1', 'key2', 'key3']),
statusText: 'OK',
},
};

mock.module('../../src/apis/api', () => ({
GET: mock(() => Promise.resolve(mockResponse)),
}));

keyValueAPI.all = async (_name: string): Promise<string[]> => {
return ['key1', 'key2', 'key3'];
};

const result = await keyValueAPI.all('test-store');

expect(result).toEqual(['key1', 'key2', 'key3']);
expect(result.length).toBe(3);
});

it('should return empty array when no keys exist', async () => {
const mockResponse = {
status: 200,
response: {
json: () => Promise.resolve([]),
statusText: 'OK',
},
};

mock.module('../../src/apis/api', () => ({
GET: mock(() => Promise.resolve(mockResponse)),
}));

keyValueAPI.all = async (_name: string): Promise<string[]> => {
return [];
};

const result = await keyValueAPI.all('empty-store');

expect(result).toEqual([]);
expect(result.length).toBe(0);
});

it('should throw an error on failed request', async () => {
const mockResponse = {
status: 500,
response: {
statusText: 'Internal Server Error',
status: 500,
},
};

mock.module('../../src/apis/api', () => ({
GET: mock(() => Promise.resolve(mockResponse)),
}));

keyValueAPI.all = async (_name: string): Promise<string[]> => {
throw new Error('Internal Server Error');
};

await expect(keyValueAPI.all('test-store')).rejects.toThrow();
});
});
});
Loading