Skip to content

Commit 40757fd

Browse files
devaladedevalade
committed
refactor: internalize execOrThrow on RemoteExecutor abstract class
Move execOrThrow from a standalone helper in setup.ts onto the RemoteExecutor abstract class so every executor (SSH, logging, fake) inherits it without duplication. Apply throughout the deploy pipeline: ReleaseManager (create/setup/switch/record), BackendStrategy (install, ecosystem write, PM2 start), and CaddyService (config write, reload). Non-zero exit codes now propagate as errors at every critical deploy step.
1 parent 7b8dc2b commit 40757fd

8 files changed

Lines changed: 57 additions & 49 deletions

File tree

‎src/cli/commands/setup.ts‎

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,13 @@ function buildTasks(executor: RemoteExecutor, config: ShipnodeConfig) {
3030
task: (_ctx: object, task: any) => task.newListr([
3131
{
3232
title: 'Update package index',
33-
task: () => execOrThrow(executor,
33+
task: () => executor.execOrThrow(
3434
'SUDO=""; [ "$EUID" -ne 0 ] && SUDO="sudo"; $SUDO apt-get update -qq',
3535
),
3636
},
3737
{
3838
title: 'Install curl git jq rsync build-essential',
39-
task: () => execOrThrow(executor,
39+
task: () => executor.execOrThrow(
4040
'SUDO=""; [ "$EUID" -ne 0 ] && SUDO="sudo"; ' +
4141
'$SUDO apt-get install -y curl git jq rsync build-essential',
4242
),
@@ -46,7 +46,7 @@ function buildTasks(executor: RemoteExecutor, config: ShipnodeConfig) {
4646
{
4747
title: 'Mise (version manager)',
4848
task: () =>
49-
execOrThrow(executor,
49+
executor.execOrThrow(
5050
`if ! command -v mise &>/dev/null; then curl -fsSL https://mise.run | sh; fi`,
5151
),
5252
},
@@ -55,11 +55,11 @@ function buildTasks(executor: RemoteExecutor, config: ShipnodeConfig) {
5555
task: (_ctx: object, task: any) => task.newListr([
5656
{
5757
title: `Install node@${nodeVersion}`,
58-
task: () => execOrThrow(executor,`${mise}; mise install -y "node@${nodeVersion}"`),
58+
task: () => executor.execOrThrow(`${mise}; mise install -y "node@${nodeVersion}"`),
5959
},
6060
{
6161
title: `Set node@${nodeVersion} as global default`,
62-
task: () => execOrThrow(executor,`${mise}; mise use -g -y "node@${nodeVersion}"`),
62+
task: () => executor.execOrThrow(`${mise}; mise use -g -y "node@${nodeVersion}"`),
6363
},
6464
], { concurrent: false }),
6565
},
@@ -68,7 +68,7 @@ function buildTasks(executor: RemoteExecutor, config: ShipnodeConfig) {
6868
task: (_ctx: object, task: any) => task.newListr([
6969
{
7070
title: 'Install pm2',
71-
task: () => execOrThrow(executor,
71+
task: () => executor.execOrThrow(
7272
`${mise}; ` +
7373
`if ! mise exec "node@${nodeVersion}" -- pm2 --version &>/dev/null; then ` +
7474
` mise exec "node@${nodeVersion}" -- npm install -g pm2; ` +
@@ -77,7 +77,7 @@ function buildTasks(executor: RemoteExecutor, config: ShipnodeConfig) {
7777
},
7878
{
7979
title: 'Configure systemd startup',
80-
task: () => execOrThrow(executor,
80+
task: () => executor.execOrThrow(
8181
`${mise}; ` +
8282
`mise exec "node@${nodeVersion}" -- pm2 startup systemd -u $USER --hp $HOME || true; ` +
8383
`mise exec "node@${nodeVersion}" -- pm2 save --force || true`,
@@ -88,7 +88,7 @@ function buildTasks(executor: RemoteExecutor, config: ShipnodeConfig) {
8888
{
8989
title: 'Caddy',
9090
task: () =>
91-
execOrThrow(executor,
91+
executor.execOrThrow(
9292
'SUDO=""; [ "$EUID" -ne 0 ] && SUDO="sudo"; ' +
9393
'if ! command -v caddy &>/dev/null; then ' +
9494
' $SUDO apt-get install -y debian-keyring debian-archive-keyring apt-transport-https; ' +
@@ -107,13 +107,13 @@ function buildTasks(executor: RemoteExecutor, config: ShipnodeConfig) {
107107
return task.newListr([
108108
{
109109
title: `Install ${db.type}`,
110-
task: () => execOrThrow(executor,buildDbInstallCommand(db)),
110+
task: () => executor.execOrThrow(buildDbInstallCommand(db)),
111111
},
112112
{
113113
title: `Create user '${db.user}' and database '${db.name}'`,
114-
task: () => execOrThrow(executor,buildDbCreateCommand(db)),
114+
task: () => executor.execOrThrow(buildDbCreateCommand(db)),
115115
},
116-
...(probe ? [{ title: 'Verify connection', task: () => execOrThrow(executor,probe) }] : []),
116+
...(probe ? [{ title: 'Verify connection', task: () => executor.execOrThrow(probe) }] : []),
117117
], { concurrent: false });
118118
},
119119
}]
@@ -127,13 +127,13 @@ function buildTasks(executor: RemoteExecutor, config: ShipnodeConfig) {
127127
return task.newListr([
128128
{
129129
title: 'Install redis-server',
130-
task: () => execOrThrow(executor,buildRedisInstallCommand(redis)),
130+
task: () => executor.execOrThrow(buildRedisInstallCommand(redis)),
131131
},
132132
...(redis.password ? [{
133133
title: 'Set password',
134-
task: () => execOrThrow(executor,buildRedisConfigureCommand(redis)),
134+
task: () => executor.execOrThrow(buildRedisConfigureCommand(redis)),
135135
}] : []),
136-
...(probe ? [{ title: 'Verify connection', task: () => execOrThrow(executor,probe) }] : []),
136+
...(probe ? [{ title: 'Verify connection', task: () => executor.execOrThrow(probe) }] : []),
137137
], { concurrent: false });
138138
},
139139
}]
@@ -143,13 +143,13 @@ function buildTasks(executor: RemoteExecutor, config: ShipnodeConfig) {
143143
task: (_ctx: object, task: any) => task.newListr([
144144
{
145145
title: `Create release structure at ${config.remotePath}`,
146-
task: () => execOrThrow(executor,
146+
task: () => executor.execOrThrow(
147147
`mkdir -p "${config.remotePath}/releases" "${config.remotePath}/shared" "${config.remotePath}/.shipnode"`,
148148
),
149149
},
150150
{
151151
title: 'Configure Caddy include',
152-
task: () => execOrThrow(executor,
152+
task: () => executor.execOrThrow(
153153
'SUDO=""; [ "$EUID" -ne 0 ] && SUDO="sudo"; ' +
154154
'$SUDO mkdir -p /etc/caddy/conf.d /var/log/caddy && ' +
155155
'grep -q "import /etc/caddy/conf.d" /etc/caddy/Caddyfile 2>/dev/null || ' +
@@ -164,13 +164,6 @@ function buildTasks(executor: RemoteExecutor, config: ShipnodeConfig) {
164164
);
165165
}
166166

167-
async function execOrThrow(executor: RemoteExecutor, cmd: string): Promise<void> {
168-
const result = await executor.exec(cmd);
169-
if (result.exitCode !== 0) {
170-
const detail = (result.stderr || result.stdout).trim();
171-
throw new Error(detail || `Command failed with exit code ${result.exitCode}`);
172-
}
173-
}
174167

175168
function sh(s: string): string {
176169
return s.replace(/'/g, "'\"'\"'");

‎src/domain/deploy/backend-strategy.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ export class BackendStrategy implements DeploymentStrategy {
7272

7373
const installResult = await ctx.executor.exec(commands.join(' && '));
7474
this.assertNoBuildScriptsIgnored(pkgManager, installResult);
75+
if (installResult.exitCode !== 0) {
76+
const detail = (installResult.stderr || installResult.stdout).trim();
77+
throw new DeployError(detail || 'Install/build failed', 'install');
78+
}
7579
}
7680

7781
async startApp(ctx: StrategyContext): Promise<void> {
@@ -82,7 +86,7 @@ export class BackendStrategy implements DeploymentStrategy {
8286
const ecosystemPath = `${this.config.remotePath}/shared/ecosystem.config.cjs`;
8387

8488
const escaped = ecosystemContent.replace(/'/g, "'\"'\"'");
85-
await ctx.executor.exec(`echo '${escaped}' > "${ecosystemPath}"`);
89+
await ctx.executor.execOrThrow(`echo '${escaped}' > "${ecosystemPath}"`);
8690

8791
const cdPath = `${this.config.remotePath}/current`;
8892
const mise = `export PATH="$HOME/.local/bin:$HOME/.local/share/mise/shims:$PATH"`;
@@ -95,9 +99,13 @@ export class BackendStrategy implements DeploymentStrategy {
9599
`cd "${cdPath}" && ${mise} && ${installCmd} --prefer-offline`,
96100
);
97101
this.assertNoBuildScriptsIgnored(pkgManager, installResult);
102+
if (installResult.exitCode !== 0) {
103+
const detail = (installResult.stderr || installResult.stdout).trim();
104+
throw new DeployError(detail || 'Package relink failed', 'start');
105+
}
98106

99107
const port = this.config.backend?.port ?? 3000;
100-
await ctx.executor.exec(
108+
await ctx.executor.execOrThrow(
101109
`cd "${cdPath}" && ${mise} && ` +
102110
`{ mise exec -- pm2 delete "${this.config.pm2!.name}" 2>/dev/null || true; } && ` +
103111
`{ ss -tlnp | grep -q ":${port} " && echo "Port ${port} is already in use by another process" && false || true; } && ` +

‎src/domain/release/manager.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,18 @@ export class ReleaseManager {
1111

1212
async createReleasePath(timestamp: string): Promise<string> {
1313
const releasePath = `${this.remotePath}/releases/${timestamp}`;
14-
await this.executor.exec(`mkdir -p "${releasePath}"`);
14+
await this.executor.execOrThrow(`mkdir -p "${releasePath}"`);
1515
return releasePath;
1616
}
1717

1818
async setupReleaseStructure(): Promise<void> {
19-
await this.executor.exec(`mkdir -p "${this.remotePath}/releases"`);
20-
await this.executor.exec(`mkdir -p "${this.remotePath}/shared"`);
21-
await this.executor.exec(`mkdir -p "${this.remotePath}/.shipnode"`);
19+
await this.executor.execOrThrow(`mkdir -p "${this.remotePath}/releases" "${this.remotePath}/shared" "${this.remotePath}/.shipnode"`);
2220
}
2321

2422
async switchSymlink(releasePath: string): Promise<void> {
2523
const tmpLink = `${this.remotePath}/current.tmp`;
26-
await this.executor.exec(`ln -sfn "${releasePath}" "${tmpLink}"`);
27-
await this.executor.exec(`mv -Tf "${tmpLink}" "${this.remotePath}/current"`);
24+
await this.executor.execOrThrow(`ln -sfn "${releasePath}" "${tmpLink}"`);
25+
await this.executor.execOrThrow(`mv -Tf "${tmpLink}" "${this.remotePath}/current"`);
2826
}
2927

3028
async recordRelease(record: ReleaseRecord): Promise<void> {
@@ -42,7 +40,7 @@ export class ReleaseManager {
4240
releases.push(record);
4341

4442
const b64 = Buffer.from(JSON.stringify(releases, null, 2)).toString('base64');
45-
await this.executor.exec(`printf '%s' '${b64}' | base64 -d > "${releasesFile}"`);
43+
await this.executor.execOrThrow(`printf '%s' '${b64}' | base64 -d > "${releasesFile}"`);
4644
}
4745

4846
async cleanupOldReleases(): Promise<void> {

‎src/domain/remote/executor.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,20 @@ import type { ExecResult } from '../../shared/types.js';
33
/**
44
* A seam for executing commands on a remote host.
55
*
6-
* Implementations satisfy this interface at the seam — callers do not
7-
* know whether the underlying transport is SSH, a local shell, or a test double.
6+
* Implementations satisfy this seam — callers do not know whether the
7+
* underlying transport is SSH, a local shell, or a test double.
88
*
9-
* The interface is intentionally narrow: a lot of behaviour (connection
10-
* management, authentication, session reuse) hides behind `exec`.
9+
* `exec` always resolves (never throws on non-zero exit). Use `execOrThrow`
10+
* when a non-zero exit should abort the calling operation.
1111
*/
12-
export interface RemoteExecutor {
13-
exec(command: string, options?: { timeout?: number }): Promise<ExecResult>;
12+
export abstract class RemoteExecutor {
13+
abstract exec(command: string, options?: { timeout?: number }): Promise<ExecResult>;
14+
15+
async execOrThrow(command: string, options?: { timeout?: number }): Promise<void> {
16+
const result = await this.exec(command, options);
17+
if (result.exitCode !== 0) {
18+
const detail = (result.stderr || result.stdout).trim();
19+
throw new Error(detail || `Command failed with exit code ${result.exitCode}`);
20+
}
21+
}
1422
}

‎src/infrastructure/ssh/connection.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,19 @@ import { join } from 'path';
44
import { Client, ConnectConfig, ClientChannel } from 'ssh2';
55
import { SshError } from '../../shared/errors.js';
66
import type { SshConfig, ExecResult } from '../../shared/types.js';
7-
import type { RemoteExecutor } from '../../domain/remote/executor.js';
7+
import { RemoteExecutor } from '../../domain/remote/executor.js';
88

99
export interface SshConnectionOptions {
1010
onReady?: () => void;
1111
onError?: (err: Error) => void;
1212
}
1313

14-
export class SshConnection implements RemoteExecutor {
14+
export class SshConnection extends RemoteExecutor {
1515
private client: Client;
1616
private connected = false;
1717

1818
constructor() {
19+
super();
1920
this.client = new Client();
2021
}
2122

‎src/infrastructure/ssh/logging-executor.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import chalk from 'chalk';
2-
import type { RemoteExecutor } from '../../domain/remote/executor.js';
2+
import { RemoteExecutor } from '../../domain/remote/executor.js';
33
import type { ExecResult } from '../../shared/types.js';
44

5-
export class LoggingExecutor implements RemoteExecutor {
6-
constructor(private inner: RemoteExecutor) {}
5+
export class LoggingExecutor extends RemoteExecutor {
6+
constructor(private inner: RemoteExecutor) { super(); }
77

88
async exec(command: string, options?: { timeout?: number }): Promise<ExecResult> {
99
const result = await this.inner.exec(command, options);

‎src/services/caddy.service.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ export class CaddyService {
1515
const caddyConfig = this.generateBackendCaddyfile(appName, port);
1616

1717
const escaped = caddyConfig.replace(/'/g, "'\"'\"'");
18-
await this.executor.exec(`echo '${escaped}' > /etc/caddy/conf.d/${appName}.caddy`);
19-
await this.executor.exec('systemctl reload caddy');
18+
await this.executor.execOrThrow(`echo '${escaped}' > /etc/caddy/conf.d/${appName}.caddy`);
19+
await this.executor.execOrThrow('systemctl reload caddy');
2020
}
2121

2222
async configureFrontend(): Promise<void> {
@@ -28,8 +28,8 @@ export class CaddyService {
2828
const caddyConfig = this.generateFrontendCaddyfile(appName, servePath);
2929

3030
const escaped = caddyConfig.replace(/'/g, "'\"'\"'");
31-
await this.executor.exec(`echo '${escaped}' > /etc/caddy/conf.d/${appName}.caddy`);
32-
await this.executor.exec('systemctl reload caddy');
31+
await this.executor.execOrThrow(`echo '${escaped}' > /etc/caddy/conf.d/${appName}.caddy`);
32+
await this.executor.execOrThrow('systemctl reload caddy');
3333
}
3434

3535
private generateBackendCaddyfile(appName: string, port: number): string {

‎tests/testing/fake-executor.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { RemoteExecutor } from '../../src/domain/remote/executor.js';
1+
import { RemoteExecutor } from '../../src/domain/remote/executor.js';
22
import type { ExecResult } from '../../src/shared/types.js';
33

44
export interface CommandLogEntry {
@@ -13,7 +13,7 @@ export interface CommandLogEntry {
1313
* Callers record expected responses with `when()`, then inspect
1414
* the command history with `getHistory()` to assert behaviour.
1515
*/
16-
export class FakeRemoteExecutor implements RemoteExecutor {
16+
export class FakeRemoteExecutor extends RemoteExecutor {
1717
private log: CommandLogEntry[] = [];
1818
private matchers: Array<{
1919
predicate: (command: string) => boolean;

0 commit comments

Comments
 (0)