🎨 Async/Await migration on bucketGet and objectGetLegalHold by DarkIsDude · Pull Request #6045 · scality/cloudserver

DarkIsDude · 2026-01-15T10:17:59Z

Description

Motivation and context

Opening a first PR to migrate from callback to async/await syntax. A TAD is opened.

https://scality.atlassian.net/wiki/spaces/OS/pages/3523346468/2025-10-30+-+Async+Await+migration

This is a first PR to start the migration. As mention in the TAD, the idea is to migrate to async/await when working on a specific part of our component. Maybe a common goal of one PR per squad per sprint is a good idea.

All idea or remarks are welcome 🙏

bert-e · 2026-01-15T10:18:03Z

Hello darkisdude,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request `TBA`
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2026-01-15T10:18:11Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-823 contains:

None

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

9.2.15
9.3.0

Please check the Fix Version/s of CLDSRV-823, or the target
branch of this pull request.

codecov · 2026-01-15T10:21:46Z

Codecov Report

❌ Patch coverage is 85.36585% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.45%. Comparing base (99b9108) to head (7b03be0).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
lib/api/objectGetLegalHold.js	82.05%	7 Missing ⚠️
lib/api/bucketGet.js	86.48%	5 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
lib/metadata/metadataUtils.js	`90.65% <100.00%> (+0.31%)`	⬆️
lib/api/bucketGet.js	`95.80% <86.48%> (-0.41%)`	⬇️
lib/api/objectGetLegalHold.js	`84.61% <82.05%> (-4.75%)`	⬇️

... and 1 file with indirect coverage changes

@@                         Coverage Diff                         @@
##           feature/CLDSRV-823/formatting-only    #6045   +/-   ##
===================================================================
  Coverage                               84.45%   84.45%           
===================================================================
  Files                                     206      206           
  Lines                                   13164    13186   +22     
===================================================================
+ Hits                                    11117    11136   +19     
- Misses                                   2047     2050    +3

Flag	Coverage Δ
file-ft-tests	`67.70% <80.48%> (+0.01%)`	⬆️
kmip-ft-tests	`28.18% <6.09%> (-0.03%)`	⬇️
mongo-v0-ft-tests	`68.91% <80.48%> (+0.01%)`	⬆️
mongo-v1-ft-tests	`69.02% <80.48%> (+0.11%)`	⬆️
multiple-backend	`35.21% <29.26%> (+<0.01%)`	⬆️
sur-tests	`35.77% <29.26%> (+<0.01%)`	⬆️
sur-tests-inflights	`37.64% <29.26%> (-0.01%)`	⬇️
unit	`69.62% <70.73%> (+0.02%)`	⬆️
utapi-v2-tests	`34.46% <29.26%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bert-e · 2026-01-16T09:06:55Z

Request integration branches

Waiting for integration branch creation to be requested by the user.

To request integration branches, please comment on this pull request with the following command:

/create_integration_branches

Alternatively, the /approve and /create_pull_requests commands will automatically
create the integration branches.

lib/api/bucketGet.js

lib/api/objectGetLegalHold.js

bert-e · 2026-01-19T14:01:21Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-823 contains:

9.2.15
9.3.0

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

9.2.16
9.3.0

Please check the Fix Version/s of CLDSRV-823, or the target
branch of this pull request.

bert-e · 2026-01-20T10:27:09Z

Request integration branches

Waiting for integration branch creation to be requested by the user.

To request integration branches, please comment on this pull request with the following command:

/create_integration_branches

Alternatively, the /approve and /create_pull_requests commands will automatically
create the integration branches.

lib/api/objectGetLegalHold.js

lib/api/bucketGet.js

lib/api/objectGetLegalHold.js

bert-e · 2026-01-22T18:05:44Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-823 contains:

9.2.18
9.3.0

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

9.2.19
9.3.0

Please check the Fix Version/s of CLDSRV-823, or the target
branch of this pull request.

bert-e · 2026-01-23T15:47:15Z

Request integration branches

Waiting for integration branch creation to be requested by the user.

To request integration branches, please comment on this pull request with the following command:

/create_integration_branches

Alternatively, the /approve and /create_pull_requests commands will automatically
create the integration branches.

bert-e · 2026-01-23T17:23:29Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-823 contains:

9.2.20
9.3.0

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

9.2.21
9.3.0

Please check the Fix Version/s of CLDSRV-823, or the target
branch of this pull request.

bert-e · 2026-01-23T18:29:57Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-823 contains:

9.2.20
9.3.0

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

9.2.22
9.3.0

Please check the Fix Version/s of CLDSRV-823, or the target
branch of this pull request.

lib/api/bucketGet.js

lib/api/objectGetLegalHold.js

lib/api/bucketGet.js

bert-e · 2026-01-29T10:53:10Z

Branches have diverged

This pull request's source branch feature/CLDSRV-823/async-await-start has diverged from
development/9.2 by more than 50 commits.

To avoid any integration risks, please re-synchronize them using one of the
following solutions:

Merge origin/development/9.2 into feature/CLDSRV-823/async-await-start
Rebase feature/CLDSRV-823/async-await-start onto origin/development/9.2

Note: If you choose to rebase, you may have to ask me to rebuild
integration branches using the reset command.

bert-e · 2026-01-29T11:04:06Z

Request integration branches

Waiting for integration branch creation to be requested by the user.

To request integration branches, please comment on this pull request with the following command:

/create_integration_branches

Alternatively, the /approve and /create_pull_requests commands will automatically
create the integration branches.

lib/api/objectGetLegalHold.js

maeldonn · 2026-01-29T17:57:20Z

lib/api/objectGetLegalHold.js


 const { convertToXml } = s3middleware.objectLegalHold;

+const standardMetadataValidateBucketAndObjPromised = (metadataValParams, actionImplicitDenies, log) =>


Instead of doing one custom promisify for each file that is migrated, we can do the same thing in another way:

https://nodejs.org/docs/latest-v22.x/api/util.html#custom-promisified-functions

Doing it at function level will facilitate its migration and reduce impacts.

we should also add this trick to the confluence page (in the tips & tricks part)

Thanks for this trick. Discover it, that's perfect for what we are doing. You can review this commit 872cfa3 and the confluence page where I added your idea 👏 https://scality.atlassian.net/wiki/spaces/OS/pages/3523346468/2025-10-30+-+Async+Await+migration#%E2%9A%A0%EF%B8%8F-Limitation%3A-Multiple-callback-arguments

lib/api/objectGetLegalHold.js

lib/api/bucketGet.js

maeldonn · 2026-01-29T18:38:24Z

lib/api/objectGetLegalHold.js

+        throw errors.NoSuchObjectLockConfiguration;
+    }
+
+    const additionalResHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket);


To keep the initial behavior, this function should be call earlier to do the err.additionalResHeader trick.

@maeldonn can you double check ? The err was not added on all errors but only one some. I rechecked and I think I followed the same path as before. Did I miss something ?

Sorry I think I confused with the bucketGet.js file... Just ignore my comment 😅

lib/api/bucketGet.js

francoisferrand · 2026-02-02T08:51:24Z

lib/api/objectGetLegalHold.js


 const { convertToXml } = s3middleware.objectLegalHold;

+const standardMetadataValidateBucketAndObjPromised = (metadataValParams, actionImplicitDenies, log) =>


we should also add this trick to the confluence page (in the tips & tricks part)

lib/api/objectGetLegalHold.js

Issue: CLDSRV-823

francoisferrand · 2026-02-05T10:59:08Z

lib/api/bucketGet.js

-    standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => {
-        const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket);
+    let error;
+    let bucket;


bucket will not be set in case standardMetadataValidateBucketPromised throws:

is it already the behavior of standardMetadataValidateBucketPromised' continuation callback, or did it always provide the bucket ? In that case promisify does not work... 😢

does collectCorsHeaders work when bucket is not provider?

DarkIsDude self-assigned this Jan 15, 2026

DarkIsDude changed the title ~~🎨 Async/Await migration~~ 🎨 Async/Await migration on bucketGet Jan 15, 2026

DarkIsDude force-pushed the feature/CLDSRV-823/async-await-start branch from 16402cf to 4e7d644 Compare January 15, 2026 12:17

DarkIsDude requested review from a team, anurag4DSB, delthas and maeldonn and removed request for a team January 16, 2026 09:03

DarkIsDude marked this pull request as ready for review January 16, 2026 09:03

DarkIsDude requested review from a team and removed request for anurag4DSB January 16, 2026 09:03

DarkIsDude changed the title ~~🎨 Async/Await migration on bucketGet~~ 🎨 Async/Await migration on bucketGet and objectGetLegalHold Jan 16, 2026

SylvainSenechal reviewed Jan 16, 2026

View reviewed changes

lib/api/bucketGet.js Outdated Show resolved Hide resolved

SylvainSenechal reviewed Jan 16, 2026

View reviewed changes

lib/api/objectGetLegalHold.js Show resolved Hide resolved

SylvainSenechal approved these changes Jan 19, 2026

View reviewed changes

delthas reviewed Jan 20, 2026

View reviewed changes

lib/api/objectGetLegalHold.js Outdated Show resolved Hide resolved

francoisferrand self-requested a review January 20, 2026 20:34

francoisferrand reviewed Jan 22, 2026

View reviewed changes

lib/api/bucketGet.js Outdated Show resolved Hide resolved

lib/api/bucketGet.js Show resolved Hide resolved

lib/api/bucketGet.js Outdated Show resolved Hide resolved

lib/api/bucketGet.js Show resolved Hide resolved

lib/api/objectGetLegalHold.js Show resolved Hide resolved

DarkIsDude force-pushed the feature/CLDSRV-823/async-await-start branch 2 times, most recently from bafa722 to 08d8dc0 Compare January 23, 2026 17:17

DarkIsDude requested review from SylvainSenechal, delthas and francoisferrand January 23, 2026 18:29

francoisferrand reviewed Jan 26, 2026

View reviewed changes

lib/api/bucketGet.js Outdated Show resolved Hide resolved

lib/api/bucketGet.js Show resolved Hide resolved

lib/api/objectGetLegalHold.js Outdated Show resolved Hide resolved

lib/api/bucketGet.js Outdated Show resolved Hide resolved

lib/api/bucketGet.js Outdated Show resolved Hide resolved

DarkIsDude force-pushed the feature/CLDSRV-823/async-await-start branch from 2196def to 250de6f Compare January 29, 2026 11:03

DarkIsDude requested a review from francoisferrand January 29, 2026 11:42

DarkIsDude force-pushed the feature/CLDSRV-823/async-await-start branch 2 times, most recently from 474bad8 to 7a14efc Compare January 29, 2026 12:14

maeldonn reviewed Jan 29, 2026

View reviewed changes

francoisferrand reviewed Feb 2, 2026

View reviewed changes

DarkIsDude force-pushed the feature/CLDSRV-823/async-await-start branch from 7a14efc to c98d577 Compare February 4, 2026 06:31

DarkIsDude changed the base branch from development/9.2 to feature/CLDSRV-823/formatting-only February 4, 2026 06:31

DarkIsDude force-pushed the feature/CLDSRV-823/async-await-start branch from c98d577 to 4f6b182 Compare February 4, 2026 06:50

DarkIsDude force-pushed the feature/CLDSRV-823/formatting-only branch from d6342e9 to 7559941 Compare February 4, 2026 06:50

DarkIsDude requested review from francoisferrand and maeldonn February 4, 2026 08:07

maeldonn approved these changes Feb 4, 2026

View reviewed changes

DarkIsDude force-pushed the feature/CLDSRV-823/formatting-only branch 3 times, most recently from 7559941 to 99b9108 Compare February 4, 2026 17:29

DarkIsDude added 4 commits February 4, 2026 18:32

♻️ Migrate bucketGet and objectGetLegalHold to async/await

757f7e6

Issue: CLDSRV-823

♻️ use trampoline function to keep backward compatibility

a24d60c

Issue: CLDSRV-823

📝 update JSDoc

d6af6e2

Issue: CLDSRV-823

♻️ factorise custom promisify function

7bed447

Issue: CLDSRV-823

DarkIsDude force-pushed the feature/CLDSRV-823/async-await-start branch from 872cfa3 to 7bed447 Compare February 4, 2026 17:32

🐛 update callback to async/await after merge

7b03be0

Issue: CLDSRV-823

francoisferrand reviewed Feb 5, 2026

View reviewed changes


		const { convertToXml } = s3middleware.objectLegalHold;

		const standardMetadataValidateBucketAndObjPromised = (metadataValParams, actionImplicitDenies, log) =>

Conversation

DarkIsDude commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and context

Uh oh!

bert-e commented Jan 15, 2026

Hello darkisdude,

Uh oh!

bert-e commented Jan 15, 2026

Incorrect fix version

Uh oh!

codecov bot commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bert-e commented Jan 16, 2026

Request integration branches

Uh oh!

Uh oh!

Uh oh!

bert-e commented Jan 19, 2026

Incorrect fix version

Uh oh!

bert-e commented Jan 20, 2026

Request integration branches

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bert-e commented Jan 22, 2026

Incorrect fix version

Uh oh!

bert-e commented Jan 23, 2026

Request integration branches

Uh oh!

bert-e commented Jan 23, 2026

Incorrect fix version

Uh oh!

bert-e commented Jan 23, 2026

Incorrect fix version

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bert-e commented Jan 29, 2026

Branches have diverged

Uh oh!

bert-e commented Jan 29, 2026

Request integration branches

Uh oh!

Uh oh!

maeldonn Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

francoisferrand Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

DarkIsDude Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maeldonn Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

DarkIsDude Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

maeldonn Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

DarkIsDude commented Jan 15, 2026 •

edited

Loading

codecov bot commented Jan 15, 2026 •

edited

Loading

francoisferrand Feb 5, 2026 •

edited

Loading