Skip to content
This repository has been archived by the owner on Nov 1, 2024. It is now read-only.

Commit

Permalink
Fix bug around handling of failures in spawnWorker calls (#87)
Browse files Browse the repository at this point in the history
Fixes #86

If a worker process fails to spawn, we will now complete the work attempt that caused the process to spawn with that failure, instead of never completing the attempt at all, causing a hang, and also leaking the async exception as an unhandled exception.

We could add retry logic in the future if we want, but it is probably unlikely that trying again will work. This way the actual failure should be reliably surfaced though.
  • Loading branch information
jakemac53 authored Mar 7, 2024
1 parent 372b8b5 commit 8619b92
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 1 deletion.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 1.1.1

* Fix a bug where if spawnWorker threw an error, work requests would hang
forever instead of failing.

## 1.1.0

* Add constructors with named parameters to
Expand Down
4 changes: 4 additions & 0 deletions lib/src/driver/driver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ class BazelWorkerDriver {
_readyWorkers.remove(worker);
_runWorkQueue();
});
}).onError<Object>((e, s) {
_spawningWorkers.remove(futureWorker);
if (attempt.responseCompleter.isCompleted) return;
attempt.responseCompleter.completeError(e, s);
});
}
// Recursively calls itself until one of the bail out conditions are met.
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: bazel_worker
version: 1.1.0
version: 1.1.1
description: >-
Protocol and utilities to implement or invoke persistent bazel workers.
repository: https://github.com/dart-lang/bazel_worker
Expand Down
6 changes: 6 additions & 0 deletions test/driver_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,12 @@ void main() {
});
});

test('handles spawnWorker failures', () async {
driver = BazelWorkerDriver(() async => throw StateError('oh no!'),
maxRetries: 0);
expect(driver!.doWork(WorkRequest()), throwsA(isA<StateError>()));
});

tearDown(() async {
await driver?.terminateWorkers();
expect(MockWorker.liveWorkers, isEmpty);
Expand Down

0 comments on commit 8619b92

Please sign in to comment.