Skip to content

Commit d6b0480

Browse files
seefeldbclaude
andauthored
fix(transactions): return NotFoundError or (new) TypeMismatchError instead of StorageTransactionInconsistent where appropriate (#1472)
* docs(runner): Add comprehensive storage transaction documentation - Add transaction-explainer.md: High-level overview for newcomers - Explains core architecture (StorageTransaction, Journal, Chronicle) - Covers key design principles (write isolation, read consistency) - Documents transaction lifecycle and usage patterns - Add transaction-implementation-guide.md: Detailed guide for maintainers - Documents state machines and transitions - Explains critical implementation patterns - Details error states and handling - Covers key algorithms (consistency checking, write merging) - Includes performance considerations and testing guidelines * feat(runner): Improve transaction error handling with distinct error types Align transaction implementation with transaction-shim.ts behavior by returning appropriate errors for different failure scenarios. Key change: StorageTransactionInconsistent errors now only occur when the underlying state has changed, maintaining the invariant that retrying after this error will see different data. This is the key indicator for when to retry a transaction. - Introduce TypeMismatchError for accessing properties on non-objects (e.g., reading string.property or null.field) - Return NotFoundError when documents or paths don't exist, instead of inconsistency errors - Fix bug where writes to nested paths of documents created in the same transaction would incorrectly fail with NotFoundError - Update all tests to expect appropriate error types - Update documentation to explain error type distinctions * feat(runner): Update transaction shim to handle TypeMismatchError gracefully The transaction shim's readOrThrow and writeOrThrow methods now treat TypeMismatchError like NotFoundError, returning undefined instead of throwing. This maintains backwards compatibility while leveraging the new distinct error types. Key changes: - validateParentPath now returns TypeMismatchError for type mismatches - readOrThrow returns undefined for both NotFoundError and TypeMismatchError - writeOrThrow handles both error types when creating parent entries - Updated tests to expect the correct error types * fix(runner): update tests for new transaction system with TypeMismatchError handling - Update storage.test.ts to work with new transaction system (shim=false) - Remove sync() calls and add explicit commits before querying - Add conditional sync() calls only when shim is true - Wait for storage processing with runtime.idle() after commits - Fix data structure expectations to match new format - Update storage-transaction-shim.test.ts expectations - Change line 309 to expect TypeMismatchError instead of NotFoundError - This aligns with the shim behavior that distinguishes type mismatches - Previously updated transaction-shim.ts to treat TypeMismatchError like NotFoundError - Both error types are now handled gracefully in readOrThrow and writeOrThrow * control whether to use new transactions via env variable * fix(runner): update tests for full document in subscription notifications Updated test expectations to match the new subscription behavior where 'before' and 'after' fields now always contain the full document rather than just the value at the specified path. Changes: - Fixed reactive-dependencies tests to pass full document structure - Updated storage-subscription tests to expect full document with wrapper - Modified storage-transaction-shim tests for consistent document structure - Adjusted expectations for empty documents to use {} instead of undefined * address PR feedback * re-add fields to INotFoundError * feat(runner): add array type checking to attestation.write() and tests Update attestation.write() to properly handle array types: - Explicitly check for arrays using Array.isArray() - Only allow writing to arrays at: - Valid non-negative integer indices (0, 1, 2, etc.) - The 'length' property for array truncation - Return TypeMismatchError for invalid array operations Add comprehensive tests to validate the new behavior: - Writing to valid positive integer indices (0, 1, 10) - Writing to array 'length' property for truncation - TypeMismatchError for negative indices (-1) - TypeMismatchError for non-integer keys (1.5) - TypeMismatchError for string keys other than 'length' - Proper handling of sparse arrays This ensures arrays are treated distinctly from objects and prevents invalid array modifications while maintaining backwards compatibility. * fix comment to reflect new names --------- 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 2f40caf commit d6b0480

19 files changed

+1401
-342
lines changed

packages/runner/src/cell.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import type {
4444
IReadOptions,
4545
} from "./storage/interface.ts";
4646
import { fromURI } from "./uri-utils.ts";
47+
import { getEntityId } from "./doc-map.ts";
4748

4849
/**
4950
* This is the regular Cell interface, generated by DocImpl.asCell().
@@ -661,10 +662,18 @@ export class RegularCell<T> implements Cell<T> {
661662
>
662663
| undefined;
663664
getSourceCell(schema?: JSONSchema): Cell<any> | undefined {
664-
const sourceCellId =
665+
let sourceCellId =
665666
(this.tx?.status().status === "ready" ? this.tx : this.runtime.edit())
666-
.readOrThrow({ ...this.link, path: ["source"] });
667-
if (!sourceCellId) return undefined;
667+
.readOrThrow({ ...this.link, path: ["source"] }) as string | undefined;
668+
if (!sourceCellId || typeof sourceCellId !== "string") {
669+
return undefined;
670+
}
671+
if (sourceCellId.startsWith('{"/":')) {
672+
sourceCellId = toURI(JSON.parse(sourceCellId));
673+
}
674+
if (!sourceCellId.startsWith("of:")) {
675+
throw new Error("Source cell ID must start with 'of:'");
676+
}
668677
return createCell(this.runtime, {
669678
space: this.link.space,
670679
path: [],
@@ -680,7 +689,10 @@ export class RegularCell<T> implements Cell<T> {
680689
if (sourceLink.path.length > 0) {
681690
throw new Error("Source cell must have empty path for now");
682691
}
683-
this.tx.writeOrThrow({ ...this.link, path: ["source"] }, sourceLink.id);
692+
this.tx.writeOrThrow(
693+
{ ...this.link, path: ["source"] },
694+
JSON.stringify(getEntityId(sourceLink.id)),
695+
);
684696
}
685697

686698
freeze(reason: string): void {

packages/runner/src/doc.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -339,14 +339,21 @@ export function createDoc<T>(
339339

340340
// Send notification if shim storage manager is available
341341
if (runtime.storage.shim) {
342+
let before = previousValue;
343+
let after = newValue;
344+
for (const key of ["value", ...path.map(String)].reverse()) {
345+
before = { [key]: before };
346+
after = { [key]: after };
347+
}
348+
342349
const change: IMemoryChange = {
343350
address: {
344351
id: toURI(entityId),
345352
type: "application/json",
346353
path: ["value", ...path.map(String)],
347354
},
348-
before: previousValue,
349-
after: newValue,
355+
before: before,
356+
after: after,
350357
};
351358

352359
const notification: ICommitNotification = {
@@ -413,8 +420,8 @@ export function createDoc<T>(
413420
type: "application/json",
414421
path: ["source"],
415422
},
416-
before: JSON.stringify(sourceCell?.entityId),
417-
after: JSON.stringify(cell?.entityId),
423+
before: { source: JSON.stringify(sourceCell?.entityId) },
424+
after: { source: JSON.stringify(cell?.entityId) },
418425
};
419426

420427
const notification: ICommitNotification = {

packages/runner/src/reactive-dependencies.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -110,17 +110,11 @@ export function determineTriggeredActions(
110110

111111
if (startPath.length > 0) {
112112
// If we're starting from a specific path, filter the subscribers to only
113-
// include those that start with that path.
113+
// include those that can be affected by that path.
114114
subscribers = subscribers.map(({ action, paths }) => ({
115115
action,
116116
paths: paths.filter((path) => arraysOverlap(path, startPath)),
117117
})).filter(({ paths }) => paths.length > 0);
118-
119-
// And prepend path to data, so we don't have to special case this.
120-
for (const key of startPath.toReversed()) {
121-
before = { [key]: before } as JSONValue;
122-
after = { [key]: after } as JSONValue;
123-
}
124118
}
125119

126120
// Sort subscribers by last/longest path first.

packages/runner/src/runtime.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { isDeno } from "@commontools/utils/env";
12
import type {
23
JSONSchema,
34
Module,
@@ -45,7 +46,9 @@ import { Runner } from "./runner.ts";
4546
import { registerBuiltins } from "./builtins/index.ts";
4647
import { StaticCache } from "@commontools/static";
4748

48-
const DEFAULT_USE_REAL_TRANSACTIONS = false;
49+
const DEFAULT_USE_REAL_TRANSACTIONS = isDeno()
50+
? ["1", "true", "on", "yes"].includes(Deno.env.get("USE_REAL_TRANSACTIONS")!)
51+
: false;
4952

5053
export type { IExtendedStorageTransaction, IStorageProvider, MemorySpace };
5154

packages/runner/src/storage/interface.ts

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -559,13 +559,13 @@ export interface ITransactionReader {
559559
* })
560560
* assert(w.ok)
561561
*
562-
* assert(tx.read({ type, id, path:: ['author'] }).ok === undefined)
563-
* assert(tx.read({ type, id, path:: ['author', 'address'] }).error.name === 'NotFoundError')
562+
* assert(tx.read({ type, id, path: ['author'] }).ok === undefined)
563+
* assert(tx.read({ type, id, path: ['author', 'address'] }).error.name === 'NotFoundError')
564564
* // JS specific getters are not supported
565-
* assert(tx.read({ the, of, at: ['content', 'length'] }).ok.is === undefined)
566-
* assert(tx.read({ the, of, at: ['title'] }).ok.is === "Hello world")
565+
* assert(tx.read({ type, id, path: ['content', 'length'] }).ok?.value === undefined)
566+
* assert(tx.read({ type, id, path: ['title'] }).ok?.value === "Hello world")
567567
* // Referencing non-existing facts produces errors
568-
* assert(tx.read({ the: 'bad/mime' , of, at: ['author'] }).error.name === 'NotFoundError')
568+
* assert(tx.read({ type: 'bad/mime', id, path: ['author'] }).error.name === 'NotFoundError')
569569
* ```
570570
*
571571
* @param address - Memory address to read from
@@ -656,7 +656,10 @@ export type CommitError =
656656

657657
export interface INotFoundError extends Error {
658658
name: "NotFoundError";
659+
source: IAttestation;
660+
address: IMemoryAddress;
659661
path?: MemoryAddressPathComponent[];
662+
from(space: MemorySpace): INotFoundError;
660663
}
661664

662665
/**
@@ -682,13 +685,15 @@ export type ReadError =
682685
| INotFoundError
683686
| InactiveTransactionError
684687
| IInvalidDataURIError
685-
| IUnsupportedMediaTypeError;
688+
| IUnsupportedMediaTypeError
689+
| ITypeMismatchError;
686690

687691
export type WriteError =
688692
| INotFoundError
689693
| IUnsupportedMediaTypeError
690694
| InactiveTransactionError
691-
| IReadOnlyAddressError;
695+
| IReadOnlyAddressError
696+
| ITypeMismatchError;
692697

693698
export type ReaderError = InactiveTransactionError;
694699

@@ -700,19 +705,6 @@ export type WriterError =
700705
export interface IStorageTransactionComplete extends Error {
701706
name: "StorageTransactionCompleteError";
702707
}
703-
export interface INotFoundError extends Error {
704-
name: "NotFoundError";
705-
706-
/**
707-
* Source in which address could not be resolved.
708-
*/
709-
source: IAttestation;
710-
711-
/**
712-
* Address that we could not resolve.
713-
*/
714-
address: IMemoryAddress;
715-
}
716708

717709
/**
718710
* Represents adddress within the memory space which is like pointer inside the
@@ -859,6 +851,28 @@ export interface IReadOnlyAddressError extends Error {
859851
from(space: MemorySpace): IReadOnlyAddressError;
860852
}
861853

854+
/**
855+
* Error returned when attempting to access a property on a non-object value.
856+
* This is different from NotFound (document doesn't exist) and Inconsistency
857+
* (state changed). This error indicates a type mismatch that would persist
858+
* even if the transaction were retried.
859+
*/
860+
export interface ITypeMismatchError extends Error {
861+
name: "TypeMismatchError";
862+
863+
/**
864+
* The address being accessed.
865+
*/
866+
address: IMemoryAddress;
867+
868+
/**
869+
* The actual type encountered.
870+
*/
871+
actualType: string;
872+
873+
from(space: MemorySpace): ITypeMismatchError;
874+
}
875+
862876
/**
863877
* Describes either observed or desired state of the memory at a specific
864878
* address.
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
# Storage Transaction Abstraction Explainer
2+
3+
The storage transaction system provides ACID-like guarantees for reading and
4+
writing data across memory spaces. Think of it as a database transaction but for
5+
a distributed, content-addressed memory system.
6+
7+
## Core Architecture
8+
9+
The transaction system is built in three layers:
10+
11+
1. **StorageTransaction**
12+
(`packages/runner/src/storage/transaction.ts:63-107`) - The main API surface
13+
that manages transaction lifecycle
14+
2. **Journal** (`packages/runner/src/storage/transaction/journal.ts:52-100`) -
15+
Tracks all reads/writes and enforces consistency
16+
3. **Chronicle**
17+
(`packages/runner/src/storage/transaction/chronicle.ts:51-223`) - Manages
18+
changes per memory space with conflict detection
19+
20+
## How It Works
21+
22+
### Transaction Lifecycle
23+
24+
A transaction moves through three states:
25+
26+
1. **Ready** - Active and accepting reads/writes
27+
2. **Pending** - Commit initiated, waiting for storage confirmation
28+
3. **Done** - Completed (successfully or with error)
29+
30+
```typescript
31+
// Create a new transaction
32+
const transaction = create(storageManager);
33+
34+
// Read and write operations
35+
const readResult = transaction.read({
36+
space: "user",
37+
id: "123",
38+
type: "application/json",
39+
path: ["name"],
40+
});
41+
const writeResult = transaction.write({
42+
space: "user",
43+
id: "123",
44+
type: "application/json",
45+
path: ["age"],
46+
}, 25);
47+
48+
// Commit changes
49+
const commitResult = await transaction.commit();
50+
```
51+
52+
### Key Design Principles
53+
54+
**1. Write Isolation**: A transaction can only write to one memory space. This
55+
prevents distributed consistency issues:
56+
57+
```typescript
58+
// First write locks the transaction to "user" space
59+
transaction.write({ space: "user", ... }, data);
60+
61+
// This will fail with WriteIsolationError
62+
transaction.write({ space: "project", ... }, data);
63+
```
64+
65+
**2. Read Consistency**: All reads capture "invariants" - assumptions about the
66+
state when read. If any invariant is violated before commit, the transaction
67+
fails:
68+
69+
```typescript
70+
// Transaction reads age = 30
71+
const age = transaction.read({ ...address, path: ["age"] });
72+
73+
// Another client changes age to 31
74+
75+
// This transaction's commit will fail due to inconsistency
76+
```
77+
78+
**3. Optimistic Updates**: Writes within a transaction see their own changes
79+
immediately:
80+
81+
```typescript
82+
transaction.write({ ...address, path: ["name"] }, "Alice");
83+
const name = transaction.read({ ...address, path: ["name"] }); // Returns "Alice"
84+
```
85+
86+
## Internal Mechanics
87+
88+
### Journal (`packages/runner/src/storage/transaction/journal.ts`)
89+
90+
The Journal manages transaction state and coordinates between readers/writers:
91+
92+
- Maintains separate Chronicle instances per memory space
93+
- Tracks all read/write activity for debugging and replay
94+
- Enforces single-writer constraint
95+
- Handles transaction abort/close lifecycle
96+
97+
### Chronicle (`packages/runner/src/storage/transaction/chronicle.ts`)
98+
99+
Each Chronicle manages changes for one memory space:
100+
101+
- **History**: Tracks all read invariants to detect conflicts
102+
- **Novelty**: Stores pending writes not yet committed
103+
- **Rebase**: Merges overlapping writes intelligently
104+
105+
Key operations:
106+
107+
1. **Read** - Checks novelty first (uncommitted writes), then replica, captures
108+
invariant
109+
2. **Write** - Validates against current state, stores in novelty
110+
3. **Commit** - Verifies all invariants still hold, builds final transaction
111+
112+
### Attestation System
113+
114+
All reads and writes produce "attestations" - immutable records of observed or
115+
desired state:
116+
117+
```typescript
118+
interface IAttestation {
119+
address: IMemoryAddress; // What was read/written
120+
value?: JSONValue; // The value (undefined = deleted)
121+
}
122+
```
123+
124+
## Error Handling
125+
126+
The system uses Result types extensively with specific errors:
127+
128+
- **TransactionCompleteError** - Operation on finished transaction
129+
- **WriteIsolationError** - Attempting to write to second space
130+
- **StorageTransactionInconsistent** - Read invariant violated (underlying state
131+
changed)
132+
- **NotFoundError** - Document or path doesn't exist
133+
- **TypeMismatchError** - Accessing properties on non-objects (e.g., trying to
134+
read `string.property`)
135+
- **ReadOnlyAddressError** - Writing to data: URIs
136+
137+
The distinction between these errors is important:
138+
139+
- **NotFoundError** is transient - the operation would succeed if the
140+
document/path existed
141+
- **TypeMismatchError** is persistent - the operation will always fail unless
142+
the data type changes
143+
- **StorageTransactionInconsistent** indicates retry is needed - the underlying
144+
data changed
145+
146+
## Advanced Features
147+
148+
**1. Data URIs**: Read-only inline data using `data:` URLs
149+
150+
**2. Path-based Access**: Read/write nested JSON paths:
151+
152+
```typescript
153+
transaction.read({ ...address, path: ["user", "profile", "name"] });
154+
```
155+
156+
**3. Activity Tracking**: All operations recorded for debugging:
157+
158+
```typescript
159+
for (const activity of transaction.journal.activity()) {
160+
// Log reads and writes
161+
}
162+
```
163+
164+
**4. Consistency Validation**: Chronicle's commit method
165+
(`packages/runner/src/storage/transaction/chronicle.ts:176-222`) carefully
166+
validates that all read invariants still hold before building the final
167+
transaction.
168+
169+
This architecture ensures strong consistency guarantees while allowing
170+
optimistic updates within a transaction boundary, making it suitable for
171+
collaborative editing scenarios where multiple clients may be modifying data
172+
concurrently.

0 commit comments

Comments
 (0)