Skip to content

Commit

Permalink
patch(fix): octo | do not commit when resource diffs have errors.
Browse files Browse the repository at this point in the history
- We now properly emit errors outside of resource transaction, which should not make any changes to the states.
- If errors are emitted during resource transaction, the states will change.
  • Loading branch information
rash805115 committed Sep 18, 2024
1 parent c94426a commit 0887a3e
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 46 deletions.
2 changes: 1 addition & 1 deletion apps/octo-docs/docs/fundamentals/actions.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ An action belongs to an entity, consists of a set of inputs, and can generate ou
Looking back at the code we wrote in the [Getting Started](/docs/getting-started/hello-world.mdx) guide,
```js
const diffs1 = await octoAws.diff(app);
const generator1 = await octoAws.beginTransaction(diffs1);
const generator1 = octoAws.beginTransaction(diffs1);
```
* The `diff()` generates the action tasks.
* A new transaction is started using `beginTransaction()`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ The `getModuleOutput()` is a helper method to collect the output from any Module
<br />

```typescript title="main.ts"
const generator = await octo.beginTransaction(app);
const generator = octo.beginTransaction(app);
const modelTransactionResult = await generator.next();
await octo.commitTransaction(app, modelTransactionResult.value);
```
Expand Down
2 changes: 1 addition & 1 deletion apps/octo-docs/docs/getting-started/hello-world.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ Please create all files in below tabs.
await octo.compose();
const app = (await octo.getModuleOutput<App>(AppModule)) as App;

const generator = await octo.beginTransaction(app);
const generator = octo.beginTransaction(app);
const modelTransactionResult = await generator.next();
await octo.commitTransaction(app, modelTransactionResult.value);
```
Expand Down
33 changes: 19 additions & 14 deletions packages/octo/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,37 +44,42 @@ export class Octo {
}: TransactionOptions = {},
): ReturnType<TransactionService['beginTransaction']> {
const diffs = await app.diff();
const transaction = await this.transactionService.beginTransaction(diffs, {
const transaction = this.transactionService.beginTransaction(diffs, {
enableResourceCapture,
yieldModelDiffs,
yieldModelDiffs: true,
yieldModelTransaction: true,
yieldResourceDiffs,
yieldResourceDiffs: true,
yieldResourceTransaction: true,
});

const modelDiffs = await transaction.next();
if (yieldModelDiffs) {
yield (await transaction.next()).value;
yield modelDiffs.value;
}

const modelTransaction = (await transaction.next()).value;
const modelTransaction = await transaction.next();
if (yieldModelTransaction) {
yield modelTransaction;
yield modelTransaction.value;
}

let resourceTransaction: DiffMetadata[][] = [];
try {
if (yieldResourceDiffs) {
yield (await transaction.next()).value;
}
const resourceDiffs = await transaction.next();
if (yieldResourceDiffs) {
yield resourceDiffs.value;
}

resourceTransaction = (await transaction.next()).value;
let resourceTransaction:
| IteratorYieldResult<DiffMetadata[][]>
| IteratorReturnResult<DiffMetadata[][]>
| undefined = undefined;
try {
resourceTransaction = await transaction.next();
if (yieldResourceTransaction) {
yield resourceTransaction;
yield resourceTransaction.value;
}

return (await transaction.next()).value;
} finally {
await this.commitTransaction(app, modelTransaction, resourceTransaction);
await this.commitTransaction(app, modelTransaction.value, resourceTransaction ? resourceTransaction.value : []);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/octo/src/modules/test-module.container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export class TestModuleContainer {
resourceDiffs: DiffMetadata[][];
resourceTransaction: DiffMetadata[][];
}> {
const generator = await this.octo.beginTransaction(app, {
const generator = this.octo.beginTransaction(app, {
enableResourceCapture: true,
yieldModelDiffs: true,
yieldModelTransaction: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe('Transaction Scenarios UT', () => {
transactionService.registerResourceActions([universalResourceAction]);

await createTestResources({ 'resource-1': [] });
const generator = await transactionService.beginTransaction([]);
const generator = transactionService.beginTransaction([]);
await generator.next();
await commitResources();
});
Expand All @@ -63,7 +63,7 @@ describe('Transaction Scenarios UT', () => {
const transactionService = await Container.get(TransactionService);

await createTestResources({ 'resource-1': [] });
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand All @@ -79,7 +79,7 @@ describe('Transaction Scenarios UT', () => {

await createTestResources({ 'resource-1': [], 'resource-2': [] });

const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand All @@ -106,14 +106,14 @@ describe('Transaction Scenarios UT', () => {

// Create a change.
await createTestResources({ 'resource-1': [], 'resource-2': [] });
const generator1 = await transactionService.beginTransaction([]);
const generator1 = transactionService.beginTransaction([]);
await generator1.next();
await commitResources();

// Revert last change.
const [, resource2] = await createTestResources({ 'resource-1': [], 'resource-2': [] });
resource2.remove();
const generator2 = await transactionService.beginTransaction([], {
const generator2 = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand Down Expand Up @@ -142,7 +142,7 @@ describe('Transaction Scenarios UT', () => {
transactionService.registerResourceActions([universalResourceAction]);

await createTestResources({ 'resource-1': [] });
const generator = await transactionService.beginTransaction([]);
const generator = transactionService.beginTransaction([]);

try {
await generator.next();
Expand All @@ -156,7 +156,7 @@ describe('Transaction Scenarios UT', () => {
const transactionService = await Container.get(TransactionService);

await createTestResources({ 'resource-1': [] });
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand All @@ -181,7 +181,7 @@ describe('Transaction Scenarios UT', () => {
const transactionService = await Container.get(TransactionService);

await createTestResources({ 'resource-1': [], 'resource-2': [] });
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand Down Expand Up @@ -212,7 +212,7 @@ describe('Transaction Scenarios UT', () => {
const transactionService = await Container.get(TransactionService);

await createTestResources({ 'resource-1': [], 'resource-2': ['resource-1'] });
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand All @@ -227,7 +227,7 @@ describe('Transaction Scenarios UT', () => {
// Modify resource-1 which is already a dirty resource.
const [resource1] = await createTestResources({ 'resource-1': [], 'resource-2': [] });
resource1.properties['key1'] = 'value1';
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand Down Expand Up @@ -260,7 +260,7 @@ describe('Transaction Scenarios UT', () => {
const transactionService = await Container.get(TransactionService);

// Not creating resource-1 is equivalent to reverting the last change in this case.
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand All @@ -275,7 +275,7 @@ describe('Transaction Scenarios UT', () => {

// Not creating resource-1 is equivalent to reverting the last change in this case.
await createTestResources({ 'resource-2': [] });
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand Down Expand Up @@ -308,14 +308,14 @@ describe('Transaction Scenarios UT', () => {

// Add resource-1.
await createTestResources({ 'resource-1': [] });
const generator1 = await transactionService.beginTransaction([]);
const generator1 = transactionService.beginTransaction([]);
await generator1.next();
await commitResources();

// Modify resource-1.
const [resource1] = await createTestResources({ 'resource-1': [] });
resource1.properties['key1'] = 'value1';
const generator2 = await transactionService.beginTransaction([]);
const generator2 = transactionService.beginTransaction([]);

try {
await generator2.next();
Expand All @@ -330,7 +330,7 @@ describe('Transaction Scenarios UT', () => {

const [resource1] = await createTestResources({ 'resource-1': [] });
resource1.properties['key1'] = 'value1';
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand Down Expand Up @@ -359,7 +359,7 @@ describe('Transaction Scenarios UT', () => {

const [resource1] = await createTestResources({ 'resource-1': [], 'resource-2': [] });
resource1.properties['key1'] = 'value1';
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand Down Expand Up @@ -394,7 +394,7 @@ describe('Transaction Scenarios UT', () => {

const [resource1] = await createTestResources({ 'resource-1': [], 'resource-2': ['resource-1'] });
resource1.properties['key1'] = 'value1';
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand All @@ -409,7 +409,7 @@ describe('Transaction Scenarios UT', () => {
// Modify resource-1 which is already a dirty resource.
const [resource1] = await createTestResources({ 'resource-1': [] });
resource1.properties['key1'] = 'value2';
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand Down Expand Up @@ -437,7 +437,7 @@ describe('Transaction Scenarios UT', () => {
const transactionService = await Container.get(TransactionService);

await createTestResources({ 'resource-1': [] });
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand All @@ -452,7 +452,7 @@ describe('Transaction Scenarios UT', () => {

// Not creating resource-1 is equivalent to reverting the last change in this case.
await createTestResources({ 'resource-1': [], 'resource-2': [] });
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand Down Expand Up @@ -485,14 +485,14 @@ describe('Transaction Scenarios UT', () => {

// Add resource-1.
await createTestResources({ 'resource-1': [] });
const generator1 = await transactionService.beginTransaction([]);
const generator1 = transactionService.beginTransaction([]);
await generator1.next();
await commitResources();

// Delete resource-1.
const [resource1] = await createTestResources({ 'resource-1': [] });
resource1.remove();
const generator2 = await transactionService.beginTransaction([]);
const generator2 = transactionService.beginTransaction([]);

try {
await generator2.next();
Expand All @@ -507,7 +507,7 @@ describe('Transaction Scenarios UT', () => {

const [resource1] = await createTestResources({ 'resource-1': [] });
resource1.remove();
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand All @@ -533,7 +533,7 @@ describe('Transaction Scenarios UT', () => {

const [resource1] = await createTestResources({ 'resource-1': [], 'resource-2': [] });
resource1.remove();
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand Down Expand Up @@ -566,7 +566,7 @@ describe('Transaction Scenarios UT', () => {
const [resource1, resource2] = await createTestResources({ 'resource-1': [], 'resource-2': [] });
resource1.remove();
resource1.addChild('resourceId', resource2, 'resourceId');
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand All @@ -581,7 +581,7 @@ describe('Transaction Scenarios UT', () => {
// Modify resource-1 which is already a dirty resource.
const [resource1] = await createTestResources({ 'resource-1': [] });
resource1.properties['key1'] = 'value2';
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand Down Expand Up @@ -609,7 +609,7 @@ describe('Transaction Scenarios UT', () => {
const transactionService = await Container.get(TransactionService);

await createTestResources({ 'resource-1': [] });
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand All @@ -624,7 +624,7 @@ describe('Transaction Scenarios UT', () => {

// Not creating resource-1 is equivalent to reverting the last change in this case.
await createTestResources({ 'resource-1': [], 'resource-2': [] });
const generator = await transactionService.beginTransaction([], {
const generator = transactionService.beginTransaction([], {
yieldResourceTransaction: true,
});

Expand Down

0 comments on commit 0887a3e

Please sign in to comment.