Skip to content

Commit

Permalink
Fix bug around handling of failures in spawnWorker calls (dart-arch…
Browse files Browse the repository at this point in the history
…ive/bazel_worker#87)

Fixes dart-lang/bazel_worker#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 38ee10a commit 8adc6fc
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 1 deletion.
5 changes: 5 additions & 0 deletions pkgs/bazel_worker/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 pkgs/bazel_worker/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 pkgs/bazel_worker/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 pkgs/bazel_worker/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 8adc6fc

Please sign in to comment.