Skip to content

Commit e59c76e

Browse files
Copilothotlong
andcommitted
Address code review feedback: fix division by zero, improve error tracking, clarify documentation
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent 686ff8a commit e59c76e

4 files changed

Lines changed: 28 additions & 11 deletions

File tree

packages/data-objectstack/README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,10 @@ dataSource.clearCache();
8383
### Cache Configuration
8484

8585
- **LRU Eviction**: Automatically evicts least recently used entries when cache is full
86-
- **TTL Expiration**: Entries expire after the configured time-to-live (default: 5 minutes)
86+
- **TTL Expiration**: Entries expire after the configured time-to-live from creation (default: 5 minutes)
87+
- Note: TTL is fixed from creation time, not sliding based on access
8788
- **Memory Limits**: Configurable maximum cache size (default: 100 entries)
88-
- **Thread-Safe**: Handles concurrent access patterns safely
89+
- **Concurrent Access**: Handles async operations safely. Note that concurrent requests for the same uncached key may result in multiple fetcher calls.
8990

9091
## Error Handling
9192

packages/data-objectstack/src/cache/MetadataCache.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,15 @@ export interface CacheStats {
3333
*
3434
* Features:
3535
* - LRU (Least Recently Used) eviction policy
36-
* - TTL (Time To Live) based expiration
36+
* - TTL (Time To Live) based expiration (fixed from creation, not sliding)
3737
* - Memory limit controls
38-
* - Thread-safe operations (async/await)
38+
* - Async-safe operations
3939
* - Performance statistics tracking
4040
*
41+
* Note: Concurrent requests for the same uncached key may result in multiple
42+
* fetcher calls. For production use cases requiring request deduplication,
43+
* consider wrapping the cache with a promise-based deduplication layer.
44+
*
4145
* @example
4246
* ```typescript
4347
* const cache = new MetadataCache({ maxSize: 100, ttl: 300000 });

packages/data-objectstack/src/errors.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,15 @@ export class BulkOperationError extends ObjectStackError {
105105
* Get a summary of the bulk operation failure
106106
*/
107107
getSummary() {
108+
const total = this.successCount + this.failureCount;
109+
const failureRate = total > 0 ? this.failureCount / total : 0;
110+
108111
return {
109112
operation: this.details.operation,
110-
total: this.successCount + this.failureCount,
113+
total: total,
111114
successful: this.successCount,
112115
failed: this.failureCount,
113-
failureRate: this.failureCount / (this.successCount + this.failureCount),
116+
failureRate: failureRate,
114117
errors: this.errors,
115118
};
116119
}

packages/data-objectstack/src/index.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,13 @@ export class ObjectStackAdapter<T = any> implements DataSource<T> {
175175
const ids = data.map(item => (item as any).id).filter(Boolean);
176176

177177
if (ids.length === 0) {
178-
throw new BulkOperationError('delete', 0, data.length,
179-
data.map((_, index) => ({ index, error: 'Missing ID' }))
180-
);
178+
// Track which items are missing IDs
179+
const errors = data.map((_, index) => ({
180+
index,
181+
error: `Missing ID for item at index ${index}`
182+
}));
183+
184+
throw new BulkOperationError('delete', 0, data.length, errors);
181185
}
182186

183187
await this.client.data.deleteMany(resource, ids);
@@ -249,12 +253,17 @@ export class ObjectStackAdapter<T = any> implements DataSource<T> {
249253
throw error;
250254
}
251255

252-
// Wrap other errors in BulkOperationError
256+
// Wrap other errors in BulkOperationError with proper error tracking
257+
const errors = data.map((_, index) => ({
258+
index,
259+
error: error.message || String(error)
260+
}));
261+
253262
throw new BulkOperationError(
254263
operation,
255264
0,
256265
data.length,
257-
[{ index: 0, error: error.message || error }],
266+
errors,
258267
{ resource, originalError: error }
259268
);
260269
}

0 commit comments

Comments
 (0)