Skip to content

Commit 446df40

Browse files
committed
Added improved plugin error handling by handling pre-mature plugin exits. Also removed plugin.kill() because it's un-necessary WIP
1 parent 4fa5682 commit 446df40

11 files changed

Lines changed: 37 additions & 52 deletions

File tree

README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ USAGE
4141
* [`codify plugins:uninstall PLUGIN...`](#codify-pluginsuninstall-plugin-1)
4242
* [`codify plugins:uninstall PLUGIN...`](#codify-pluginsuninstall-plugin-2)
4343
* [`codify plugins update`](#codify-plugins-update)
44-
* [`codify uninstall [RESOURCE]`](#codify-uninstall-resource)
44+
* [`codify uninstall RESOURCES`](#codify-uninstall-resources)
4545

4646
## `codify apply [FILE]`
4747

@@ -402,16 +402,16 @@ DESCRIPTION
402402

403403
_See code: [@oclif/plugin-plugins](https://github.com/oclif/plugin-plugins/blob/v3.9.4/src/commands/plugins/update.ts)_
404404

405-
## `codify uninstall [RESOURCE]`
405+
## `codify uninstall RESOURCES`
406406

407407
describe the command here
408408

409409
```
410410
USAGE
411-
$ codify uninstall [RESOURCE]
411+
$ codify uninstall RESOURCES...
412412
413413
ARGUMENTS
414-
RESOURCE resource type to uninstall
414+
RESOURCES... A resource typeId for uninstalling. Ex: "codify uninstall homebrew"
415415
416416
DESCRIPTION
417417
describe the command here

codify.json

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@
22
{
33
"type": "project",
44
"plugins": {
5-
"default": "/Users/kevinwang/Projects/codify2/homebrew-plugin/src/index.ts"
5+
"default": "../homebrew-plugin"
66
}
77
},
88
{
99
"type": "nvm",
1010
"global": "18.20",
1111
"nodeVersions": [
12-
"18.20.2"
12+
"18.20"
1313
]
1414
},
1515
{
@@ -18,5 +18,13 @@
1818
"cirruslabs/cli/cirrus",
1919
"cirruslabs/cli/tart"
2020
]
21+
},
22+
{
23+
"type": "vscode"
24+
},
25+
{
26+
"type": "aws-configure",
27+
"profile": "codify",
28+
"csvCredentials": "../../../Downloads/rootkey.csv"
2129
}
2230
]

src/commands/apply/index.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,12 @@ export default class Apply extends Command {
3333

3434
const resolvedPath = path.resolve(flags.path ?? '.');
3535

36-
const planResult = await PlanOrchestrator.run(resolvedPath, false);
36+
const planResult = await PlanOrchestrator.run(resolvedPath);
3737
reporter.displayPlan(planResult.plan);
3838

3939
// Short circuit and exit if every change is NOOP
4040
if (planResult.plan.every((p) => p.operation === ResourceOperation.NOOP)) {
4141
console.log('No changes necessary. Exiting');
42-
await planResult.pluginCollection.destroy();
4342
return process.exit(0);
4443
}
4544

src/orchestrators/apply.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,7 @@ export const ApplyOrchestrator = {
1010
.filter((p) => p.operation !== ResourceOperation.NOOP)
1111

1212
ctx.processStarted(ProcessName.APPLY);
13-
1413
await pluginCollection.apply(filteredPlan);
15-
await pluginCollection.destroy();
16-
1714
ctx.processFinished(ProcessName.APPLY);
1815
},
1916
};

src/orchestrators/common.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,5 @@ export const CommonOrchestrator = {
1313
ctx.subprocessFinished(SubProcessName.INITIALIZE_PLUGINS)
1414

1515
return { dependencyMap, pluginCollection };
16-
},
16+
}
1717
};

src/orchestrators/plan.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { Project } from '../entities/project.js';
44
import { ctx, ProcessName, SubProcessName } from '../events/context.js';
55
import { Parser } from '../parser/index.js';
66
import { PluginCollection } from '../plugins/plugin-collection.js';
7+
import { createStartupShellScriptsIfNotExists } from '../utils/file.js';
78
import { CommonOrchestrator } from './common.js';
89

910
export interface PlanOrchestratorResponse {
@@ -13,14 +14,15 @@ export interface PlanOrchestratorResponse {
1314
}
1415

1516
export const PlanOrchestrator = {
16-
async run(path: string, destroyPlugins = true): Promise<PlanOrchestratorResponse> {
17+
async run(path: string): Promise<PlanOrchestratorResponse> {
1718
ctx.processStarted(ProcessName.PLAN)
1819

1920
ctx.subprocessStarted(SubProcessName.PARSE);
2021
const project = await Parser.parseProject(path);
2122
ctx.subprocessFinished(SubProcessName.PARSE);
2223

2324
const { dependencyMap, pluginCollection } = await CommonOrchestrator.initializePlugins(project);
25+
await createStartupShellScriptsIfNotExists();
2426

2527
ctx.subprocessStarted(SubProcessName.VALIDATE)
2628
project.validateWithResourceMap(dependencyMap);
@@ -36,10 +38,6 @@ export const PlanOrchestrator = {
3638
const plan = await pluginCollection.getPlan(project);
3739
ctx.subprocessFinished(SubProcessName.GENERATE_PLAN)
3840

39-
if (destroyPlugins) {
40-
await pluginCollection.destroy();
41-
}
42-
4341
ctx.processFinished(ProcessName.PLAN)
4442

4543
return {

src/orchestrators/uninstall.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { PlanResponseData, ResourceOperation } from 'codify-schemas';
22
import { randomUUID } from 'node:crypto';
33

44
import { ctx, ProcessName } from '../events/context.js';
5+
import { createStartupShellScriptsIfNotExists } from '../utils/file.js';
56
import { CommonOrchestrator } from './common.js';
67

78
export const UninstallOrchestrator = {
@@ -18,12 +19,10 @@ export const UninstallOrchestrator = {
1819
} as PlanResponseData))
1920

2021
const { pluginCollection } = await CommonOrchestrator.initializePlugins();
22+
await createStartupShellScriptsIfNotExists();
2123

2224
ctx.processStarted(ProcessName.UNINSTALL);
23-
2425
await pluginCollection.apply(plan);
25-
await pluginCollection.destroy();
26-
2726
ctx.processFinished(ProcessName.UNINSTALL);
2827
},
2928
};

src/plugins/plugin-collection.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,6 @@ export class PluginCollection {
7878
}
7979
}
8080

81-
async destroy(): Promise<void> {
82-
for (const plugin of this.plugins.values()) {
83-
plugin.destroy();
84-
}
85-
}
86-
8781
private async resolvePlugins(project?: Project): Promise<Plugin[]> {
8882
const pluginDefinitions: Record<string, string> = {
8983
...DEFAULT_PLUGINS,

src/plugins/plugin-process.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,13 @@ export class PluginProcess {
4141

4242
_process.stdout!.on('data', (message) => ctx.pluginStdout(message.toString('utf8')));
4343
_process.stderr!.on('data', (message) => ctx.pluginStderr(message.toString('utf8')));
44-
44+
_process.on('exit', (code) => {
45+
throw new Error(`Plugin ${this.name} exited with code ${code}`);
46+
})
4547

4648
return new PluginProcess(_process);
4749
}
4850

49-
killPlugin(): void {
50-
this.process.kill();
51-
}
52-
5351
async sendMessageForResult(message: PluginMessage): Promise<unknown> {
5452
return new Promise((resolve, reject) => {
5553
const handler = new SendMessageForResultHandler(message, this.process, resolve, reject);

src/plugins/plugin.ts

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export class Plugin {
5151
const response = await this.process!.sendMessageForResult({ cmd: 'validate', data: { configs: rawConfigs } });
5252

5353
if (!this.validateValidateResponse(response)) {
54-
throw new Error(`Invalid validate response from plugin: ${this.name}`);
54+
throw new Error(`Plugin error: Invalid validate response from plugin: ${this.name}`);
5555
}
5656

5757
return response;
@@ -61,7 +61,7 @@ export class Plugin {
6161
const response = await this.process!.sendMessageForResult({ cmd: 'plan', data: resource.raw });
6262

6363
if (!this.validatePlanResponse(response)) {
64-
throw new Error(`Plugin error: plugin ${this.name} returned invalid plan response`)
64+
throw new Error(`Plugin error: plugin ${this.name} returned invalid plan response: ${JSON.stringify(planResponseValidator.errors, null, 2)}`)
6565
}
6666

6767
return response;
@@ -71,31 +71,15 @@ export class Plugin {
7171
await this.process!.sendMessageForResult({ cmd: 'apply', data: { plan } });
7272
}
7373

74-
destroy() {
75-
this.process!.killPlugin();
76-
}
77-
7874
private validateInitializeResponse(response: unknown): response is InitializeResponseData {
79-
if (!initializeResponseValidator(response)) {
80-
throw new Error(`Invalid initialize response from plugin: ${this.name}. Error: ${initializeResponseValidator.errors}`)
81-
}
82-
83-
return true;
75+
return initializeResponseValidator(response)
8476
}
8577

8678
private validateValidateResponse(response: unknown): response is ValidateResponseData {
87-
if (!validateResponseValidator(response)) {
88-
throw new Error(`Invalid validate response from plugin: ${this.name}. Error: ${initializeResponseValidator.errors}`)
89-
}
90-
91-
return true;
79+
return validateResponseValidator(response)
9280
}
9381

9482
private validatePlanResponse(response: unknown): response is PlanResponseData {
95-
if (!planResponseValidator(response)) {
96-
throw new Error(`Invalid plan response from plugin: ${this.name}. Error: ${initializeResponseValidator.errors}`)
97-
}
98-
99-
return true;
83+
return planResponseValidator(response);
10084
}
10185
}

0 commit comments

Comments
 (0)