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();
}

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
44.1% Coverage on New Code (required ≥ 80%)
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

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