Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 9db0093

Browse files
committedDec 27, 2024··
Make allSettled usage safer
allSettled does not follow the usuage fullfil pattern: it will never reject, and always fullfil with an array of the results of each promises. This is not an issue in the case of lifecycle, where we actually ignore all errors; but it makes the code look inconsistent, as it suggests errors are possible but not handle them. To avoid future issues, add proper processing of the results of allSettled to build a single error when appropriate. Issue: BB-641
1 parent 77f6a9b commit 9db0093

File tree

2 files changed

+12
-6
lines changed

2 files changed

+12
-6
lines changed
 

‎extensions/lifecycle/tasks/LifecycleTask.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ class LifecycleTask extends BackbeatTask {
429429

430430
// if no versions to process, skip further processing for this batch
431431
if (allVersionsWithStaleDate.length === 0) {
432-
return Promise.allSettled(promises).then(() => done(), done);
432+
return Promise.allSettled(promises).then(() => done());
433433
}
434434

435435
// for each version, get their relative rules, compare with
@@ -457,7 +457,9 @@ class LifecycleTask extends BackbeatTask {
457457
return resolve();
458458
})));
459459

460-
return Promise.allSettled(promises).then(() => done(), done);
460+
return Promise.allSettled(promises).then(results => done(
461+
results.findLast(r => r.status === 'rejected')?.reason
462+
));
461463
});
462464
}
463465

‎extensions/lifecycle/tasks/LifecycleTaskV2.js

+8-4
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ class LifecycleTaskV2 extends LifecycleTask {
114114
return this.backbeatMetadataProxy.listLifecycle(listType, params, log,
115115
(err, contents, isTruncated, markerInfo) => {
116116
if (err) {
117-
return Promise.allSettled(promises).then(() => done(err), () => done(err));
117+
return Promise.allSettled(promises).then(() => done(err));
118118
}
119119

120120
// re-queue truncated listing only once.
@@ -145,7 +145,9 @@ class LifecycleTaskV2 extends LifecycleTask {
145145
bucketData, bucketLCRules, contents, log,
146146
));
147147

148-
return Promise.allSettled(promises).then(() => done(), done);
148+
return Promise.allSettled(promises).then(results => done(
149+
results.findLast(r => r.status === 'rejected')?.reason
150+
));
149151
});
150152
}
151153

@@ -195,7 +197,7 @@ class LifecycleTaskV2 extends LifecycleTask {
195197
return this.backbeatMetadataProxy.listLifecycle(listType, params, log,
196198
(err, contents, isTruncated, markerInfo) => {
197199
if (err) {
198-
return Promise.allSettled(promises).then(() => done(err), () => done(err));
200+
return Promise.allSettled(promises).then(() => done(err));
199201
}
200202

201203
// create Set of unique keys not matching the next marker to
@@ -255,7 +257,9 @@ class LifecycleTaskV2 extends LifecycleTask {
255257
return resolve();
256258
})));
257259

258-
return Promise.allSettled(promises).then(() => done(), done);
260+
return Promise.allSettled(promises).then(results => done(
261+
results.findLast(r => r.status === 'rejected')?.reason
262+
));
259263
});
260264
}
261265

0 commit comments

Comments
 (0)
Please sign in to comment.