Skip to content

Fix common API error with promise reject#1614

Merged
prxt6529 merged 2 commits intomainfrom
fix-common-api-error-reject-promise
Nov 21, 2025
Merged

Fix common API error with promise reject#1614
prxt6529 merged 2 commits intomainfrom
fix-common-api-error-reject-promise

Conversation

@prxt6529
Copy link
Copy Markdown
Collaborator

@prxt6529 prxt6529 commented Nov 21, 2025

Summary by CodeRabbit

  • Tests

    • Updated tests to assert promise rejections by exact rejection value instead of matching thrown error messages.
  • Refactor

    • API error handling now propagates failures as Promise rejections carrying the error message string for more consistent promise-based error flow.

✏️ Tip: You can customize this high-level summary in your review settings.

Signed-off-by: prxt6529 <prxt@6529.io>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 21, 2025

Walkthrough

Error handling in the API layer was changed to return Promise rejections with error message strings instead of throwing Error objects; tests were updated to assert rejected values directly.

Changes

Cohort / File(s) Change Summary
Error Handling Logic
services/api/common-api.ts
handleApiError now returns Promise.reject(errorMessage) (rejects with a string) instead of throwing an Error object.
Test Assertion Updates
__tests__/services-common-api*, __tests__/services/common-api.postNoBody.test.ts, __tests__/services/common-api.test.ts
Updated test expectations from toThrow("err") to toBe("err") to assert the rejection value equals the error message string.

Sequence Diagram(s)

sequenceDiagram
    participant Client as API caller
    participant Handler as handleApiError
    participant Promise as Promise chain

    rect rgb(250,250,255)
    Note over Handler: Previous flow (throw)
    Client->>Handler: API error detected
    Handler->>Handler: throw new Error(msg)
    Handler-->>Promise: exception thrown (Error object)
    end

    rect rgb(240,255,240)
    Note over Handler: New flow (reject)
    Client->>Handler: API error detected
    Handler->>Handler: return Promise.reject(msg)
    Handler-->>Promise: rejected with string
    Promise-->>Client: rejection handled by callers/tests
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Pay attention to callers of executeApiRequest / handleApiError that may expect Error instances (stack/message properties).
  • Verify test coverage for other codepaths that may rely on thrown Error objects rather than rejection strings.

Possibly related PRs

Suggested reviewers

  • simo6529

Poem

🐰 I hopped through code under moonlit strings,
Promises now reject with simpler things.
No Error fur to clean or fray,
Just tidy messages to brighten the day. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: converting error handling in the common API from throwing errors to returning rejected promises with error messages.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-common-api-error-reject-promise

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c468bb3 and a3dd0a0.

📒 Files selected for processing (1)
  • services/api/common-api.ts (1 hunks)
🧰 Additional context used
🪛 GitHub Check: SonarCloud Code Analysis
services/api/common-api.ts

[failure] 55-55: Prefer throw error over return Promise.reject(error).

See more on https://sonarcloud.io/project/issues?id=6529-Collections_6529seize-frontend&issues=AZqlvItZ9MqIcEQ4SFAX&open=AZqlvItZ9MqIcEQ4SFAX&pullRequest=1614


[warning] 55-55: Expected the Promise rejection reason to be an Error.

See more on https://sonarcloud.io/project/issues?id=6529-Collections_6529seize-frontend&issues=AZqlvItZ9MqIcEQ4SFAW&open=AZqlvItZ9MqIcEQ4SFAW&pullRequest=1614

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (javascript-typescript)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Signed-off-by: prxt6529 <prxt@6529.io>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f83bba6 and c468bb3.

📒 Files selected for processing (3)
  • __tests__/services/common-api.postNoBody.test.ts (1 hunks)
  • __tests__/services/common-api.test.ts (2 hunks)
  • services/api/common-api.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
__tests__/services/common-api.test.ts (2)
services/api/common-api.ts (2)
  • commonApiFetch (86-111)
  • commonApiPost (232-251)
__mocks__/@/services/api/common-api.ts (2)
  • commonApiFetch (11-11)
  • commonApiPost (13-13)
🪛 GitHub Check: SonarCloud Code Analysis
services/api/common-api.ts

[warning] 56-56: Expected the Promise rejection reason to be an Error.

See more on https://sonarcloud.io/project/issues?id=6529-Collections_6529seize-frontend&issues=AZqluXSEmrVyPw1xXFTB&open=AZqluXSEmrVyPw1xXFTB&pullRequest=1614


[warning] 55-55: Replace this trivial promise with "Promise.reject".

See more on https://sonarcloud.io/project/issues?id=6529-Collections_6529seize-frontend&issues=AZqluXSEmrVyPw1xXFTA&open=AZqluXSEmrVyPw1xXFTA&pullRequest=1614

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
__tests__/services/common-api.test.ts (2)

53-53: Test assertion correctly updated.

The test now correctly expects the promise to reject with the string value "err" rather than throwing an Error object, aligning with the implementation change in handleApiError.


106-108: Test assertion correctly updated.

The test now correctly expects the promise to reject with the string value "err" rather than throwing an Error object, consistent with the new error handling behavior.

__tests__/services/common-api.postNoBody.test.ts (1)

41-41: Test assertion correctly updated.

The test now correctly expects the promise to reject with the string value "err", matching the updated error handling semantics in handleApiError.

Comment thread services/api/common-api.ts Outdated
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Nov 21, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
2 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@prxt6529 prxt6529 merged commit 0944d15 into main Nov 21, 2025
10 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Apr 22, 2026
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.

2 participants