Skip to content

test(sdk): build the fetch stub's promise with Promise.try - #288

Merged
asafgolombek merged 1 commit into
mainfrom
dev/asaf/sonar-promise-try
Oct 5, 2026
Merged

asafgolombek merged 1 commit into
mainfrom
dev/asaf/sonar-promise-try

Conversation

@asafgolombek

@asafgolombek asafgolombek commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Clears the one Sonar issue open on main: S4634 ("replace this trivial promise with Promise.resolve") in sdks/typescript/src/test-support/global-fetch-stub.ts. #287 wrote that executor to clear S7503; Sonar then read it as a trivial promise.

The executor exists only so a throwing handler rejects instead of throwing at the call, as the real fetch does. Promise.try(handler, url, init) keeps exactly that behaviour: it calls the handler at once and turns a throw into a rejection, with no executor. Sonar's own suggestion, Promise.resolve(handler(...)), would not keep it.

Verification

  • bun run typecheck and bun run lint: clean. The SDK targets ESNext with TypeScript 7, and Bun 1.3.14 has Promise.try.
  • bun test sdks/typescript/src/test-support sdks/typescript/src/connector-kit: 127 pass, 0 fail.
  • Red-proven: with Promise.resolve(handler(url, init)) in its place, the test "a throwing handler rejects the returned promise instead of throwing at the call" fails.

Test-support code only: it is excluded from tsconfig.build.json, so nothing published changes.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Updated how the fetch test stub handles synchronous errors; they continue to be returned as rejected promises.

Sonar's S4634 flagged the executor #287 wrote to clear S7503: a
new Promise whose executor only calls resolve reads as a trivial
promise. The executor existed only so a throwing handler rejects
instead of throwing at the call. Promise.try does exactly that, calling
the handler at once and turning a throw into a rejection, with no
executor. Promise.resolve(handler(...)) would not: the throw escapes
the call, and the throwing-handler test fails with it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 5, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Repository: nimbus-agent/nimbus-sdk/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 41e6bf6d-7425-4459-a71b-3325fbd507b3
📥 Commits

Reviewing files that changed from the base of the PR and between abf218d and 2ce9242.

📒 Files selected for processing (1)
  • sdks/typescript/src/test-support/global-fetch-stub.ts
 ___________________________________
< Faking self-awareness since 2023. >
 -----------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@sonarqubecloud

sonarqubecloud Bot commented Oct 5, 2026

Copy link
Copy Markdown

@asafgolombek
asafgolombek merged commit 5a23833 into main Oct 5, 2026
27 of 28 checks passed
@asafgolombek
asafgolombek deleted the dev/asaf/sonar-promise-try branch October 5, 2026 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant