Skip to content

Fix common api error thrown#1613

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

Fix common api error thrown#1613
prxt6529 merged 2 commits intomainfrom
fix-common-api-error

Conversation

@prxt6529
Copy link
Copy Markdown
Collaborator

@prxt6529 prxt6529 commented Nov 21, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Improved error messages for batch NFT transfers to surface more detailed error text when available.
    • Simplified API error messages by removing redundant HTTP status info for clearer, concise reporting.
  • Tests

    • Updated test expectations to match the simplified error message content.

✏️ 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-message construction was simplified: ERC1155 batch-transfer errors now prefer e.details, and API error strings no longer include HTTP status/statusText. These edits only change error text content; control flow and success/error state transitions remain unchanged.

Changes

Cohort / File(s) Change Summary
Error message simplification
components/nft-transfer/TransferModal.tsx, services/api/common-api.ts
TransferModal.tsx prefers e.details when building batch ERC1155 transfer error messages; common-api.ts removes HTTP status and statusText from thrown error strings. Both changes alter only the observable error text, not control flow.
Test adjustments
__tests__/services/common-api.postNoBody.test.ts, __tests__/services/common-api.test.ts
Tests relaxed assertions to expect the error body content (e.g., "err") instead of full formatted strings containing HTTP status or reason.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Check TransferModal.tsx fallback handling covers all expected error shapes (e.g., details, shortMessage, message, raw error).
  • Verify any logging or consumers that previously relied on HTTP status in common-api.ts are not broken by the removed status text.
  • Confirm updated tests accurately reflect intended error-message behavior.

Poem

🐰 I nibbled at a tangled trace,

Found details hiding in a place.
Status numbers hopped away—
Now messages are bright as day.
A tiny hop, a tidy cheer!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix common api error thrown' is directly related to the main changes in the pull request, which modify error handling in the common API service.
✨ 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

📜 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 0dd389b and 56e5a37.

📒 Files selected for processing (2)
  • __tests__/services/common-api.postNoBody.test.ts (1 hunks)
  • __tests__/services/common-api.test.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
__tests__/services/common-api.test.ts (2)
services/api/common-api.ts (1)
  • commonApiFetch (84-109)
__mocks__/@/services/api/common-api.ts (1)
  • commonApiFetch (11-11)
⏰ 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.postNoBody.test.ts (1)

32-42: Assertion now correctly reflects simplified error message

The test’s expectation of .rejects.toThrow("err") matches the new behavior of throwing just the error body text, while still verifying rejection on non‑OK responses. No issues spotted.

__tests__/services/common-api.test.ts (2)

43-61: Fetch error-path test aligned with new error format

The expectation on Line 53 checks for .rejects.toThrow("err"), which is consistent with the updated API error handling that surfaces only the body’s error text. Structure and mocking look correct.


96-119: Post error-path test also correctly updated

The commonApiPost error test now asserts .rejects.toThrow("err") (Line 107), matching the new standardized error message behavior and using a mock response consistent with the fetch test. Looks good.


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.

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: 0

🧹 Nitpick comments (1)
components/nft-transfer/TransferModal.tsx (1)

1321-1321: Consider consistent error detail extraction across transfer types.

For consistency with the batch ERC1155 error handling (lines 1263-1265), consider prioritizing details in the error fallback chain here as well. Both ERC721 and ERC1155 transfers interact with similar blockchain error structures.

Apply this diff to line 1321:

-                        error: String(e?.shortMessage || e?.message || e),
+                        error: String(
+                          e?.details ?? (e?.shortMessage || e?.message || e)
+                        ),

Apply this diff to line 1335:

-                      error: String(
-                        error?.shortMessage || error?.message || error
-                      ),
+                      error: String(
+                        error?.details ?? (error?.shortMessage || error?.message || error)
+                      ),

Also applies to: 1335-1335

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ae05e8 and 0dd389b.

📒 Files selected for processing (2)
  • components/nft-transfer/TransferModal.tsx (1 hunks)
  • services/api/common-api.ts (1 hunks)
⏰ 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 (2)
components/nft-transfer/TransferModal.tsx (1)

1263-1265: LGTM! Enhanced error detail extraction.

The addition of e?.details as the first priority in the error fallback chain improves error reporting for batch ERC1155 transfers. This complements the simplified API error format and ensures more informative details are surfaced when available.

services/api/common-api.ts (1)

55-55: LGTM! Simplified error message format.

The removal of HTTP status information from the error message makes errors more user-friendly and consistent. The errorMessage already contains meaningful content extracted from the response body or statusText, which should be sufficient for most error reporting needs.

Note: HTTP status codes can be useful for debugging, but this trade-off appears intentional for better UX.

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

@prxt6529 prxt6529 merged commit f83bba6 into main Nov 21, 2025
8 checks passed
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