Skip to content

fix: added safety promise checks in redis, mongodb#2363

Open
abhilash-sivan wants to merge 3 commits intomainfrom
fix-safety-on-promise
Open

fix: added safety promise checks in redis, mongodb#2363
abhilash-sivan wants to merge 3 commits intomainfrom
fix-safety-on-promise

Conversation

@abhilash-sivan
Copy link
Contributor

@abhilash-sivan abhilash-sivan marked this pull request as ready for review February 24, 2026 09:37
@abhilash-sivan abhilash-sivan requested a review from a team as a code owner February 24, 2026 09:37
);
} else {
tracingUtil.handleUnexpectedReturnValue(command.promise, exports.spanName, `command "${command.name}"`);
callback(null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔

Do we have to do

callback = cls.ns.bind(onResult);

if

  if (typeof command.promise?.then === 'function') {

is false?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added callback(null) for finishing the span.

The callback is bound with onResult function which finishes and transmits the span.

I agree; simply calling onResult() is much better

tracingUtil.handleUnexpectedReturnValue(resultPromise, exports.spanName, 'command');

span.d = Date.now() - span.ts;
span.transmit();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we follow the finishSpan pattern?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we can adapt that and use:

function finishSpan () {
    span.d = Date.now() - span.ts;
    span.transmit();
}

@abhilash-sivan abhilash-sivan force-pushed the fix-safety-on-promise branch from d1beec0 to 5cba6fd Compare March 2, 2026 07:35
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 2, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
42.9% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants