Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix service behavior after dependency failure in watch mode #596

Merged
merged 7 commits into from
Dec 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic
Versioning](https://semver.org/spec/v2.0.0.html).

<!-- ## [Unreleased] -->
## [Unreleased]

### Fixed

- Fixed bug relating to services not getting shut down following an error in one
of its dependencies.
- Fixed some cases of errors being logged multiple times.
- Errors are now consistently logged immediately when they occur, instead of
sometimes only at the end of all execution.

## [0.9.1] - 2022-12-06

Expand Down
6 changes: 0 additions & 6 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,6 @@ const run = async (): Promise<Result<void, Failure[]>> => {
errors.push(result.error);
}
}
if (errors.length > 0) {
return {
ok: false,
error: errors,
};
}
}
return errors.length === 0
? {ok: true, value: undefined}
Expand Down
1 change: 1 addition & 0 deletions src/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export type Failure =
interface ErrorBase<T extends PackageReference = ScriptReference>
extends EventBase<T> {
type: 'failure';
logged?: true;
}

/**
Expand Down
222 changes: 144 additions & 78 deletions src/execution/service.ts

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/execution/standard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ export class StandardScriptExecution extends BaseExecutionWithCommand<StandardSc
});
}
} else {
this._logger.log(result.error);
// This failure will propagate to the Executor eventually anyway, but
// asynchronously.
//
Expand Down
4 changes: 4 additions & 0 deletions src/logging/default-logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ export class DefaultLogger implements Logger {
}

case 'failure': {
if (event.logged) {
return;
}
event.logged = true;
const reason = event.reason;
switch (reason) {
default: {
Expand Down
98 changes: 98 additions & 0 deletions src/test/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1275,4 +1275,102 @@ test(
})
);

test(
'service watch mode recovery from dependency failure',
// service
// |
// v
// standard
timeout(async ({rig}) => {
const service = await rig.newCommand();
const standard = await rig.newCommand();
await rig.writeAtomic({
'package.json': {
scripts: {
service: 'wireit',
standard: 'wireit',
},
wireit: {
service: {
command: service.command,
service: true,
dependencies: ['standard'],
},
standard: {
command: standard.command,
files: ['input'],
},
},
},
});

await rig.write('input', '1');
const wireit = rig.exec('npm run service --watch');

// Initial build is OK.
await wireit.waitForLog(/\[standard\] Running/);
(await standard.nextInvocation()).exit(0);
await wireit.waitForLog(/\[standard\] Executed successfully/);
await service.nextInvocation();
await wireit.waitForLog(/\[service\] Service started/);
await wireit.waitForLog(/\[service\] Watching for file changes/);
await new Promise((resolve) => setTimeout(resolve, 50));
assert.equal(service.numInvocations, 1);
assert.equal(standard.numInvocations, 1);

// Introduce an error. Service keeps running but goes into a temporary
// "started-broken" state, where it awaits its dependencies being fixed.
await rig.write('input', '2');
await wireit.waitForLog(/\[standard\] Running/);
(await standard.nextInvocation()).exit(1);
await wireit.waitForLog(/\[standard\] Failed with exit status 1/);
await wireit.waitForLog(/\[service\] Watching for file changes/);
await new Promise((resolve) => setTimeout(resolve, 50));
assert.equal(service.numInvocations, 1);
assert.equal(standard.numInvocations, 2);

// Fix the error. Service restarts because the fingerprint of its dependency
// has changed.
await rig.write('input', '3');
await wireit.waitForLog(/\[standard\] Running/);
(await standard.nextInvocation()).exit(0);
await wireit.waitForLog(/\[standard\] Executed successfully/);
await service.nextInvocation();
await wireit.waitForLog(/\[service\] Service stopped/);
await wireit.waitForLog(/\[service\] Service started/);
await wireit.waitForLog(/\[service\] Watching for file changes/);
await new Promise((resolve) => setTimeout(resolve, 50));
assert.equal(service.numInvocations, 2);
assert.equal(standard.numInvocations, 3);

// Introduce another error. Again the service keeps running as
// "started-broken".
await rig.write('input', '4');
await wireit.waitForLog(/\[standard\] Running/);
(await standard.nextInvocation()).exit(1);
await wireit.waitForLog(/\[standard\] Failed with exit status 1/);
await wireit.waitForLog(/\[service\] Watching for file changes/);
await new Promise((resolve) => setTimeout(resolve, 50));
assert.equal(service.numInvocations, 2);
assert.equal(standard.numInvocations, 4);

// Fix the error, this time by reverting. This time the service doesn't
// restart, because the fingerprint has been restored to what it was before
// the failure.
await rig.write('input', '3');
await wireit.waitForLog(/\[standard\] Running/);
(await standard.nextInvocation()).exit(0);
await wireit.waitForLog(/\[standard\] Executed successfully/);
await wireit.waitForLog(/\[service\] Watching for file changes/);
await new Promise((resolve) => setTimeout(resolve, 50));
assert.equal(service.numInvocations, 2);
assert.equal(standard.numInvocations, 5);

wireit.kill();
await wireit.exit;
assert.equal(service.numInvocations, 2);
assert.equal(standard.numInvocations, 5);
})
);

test.run();