Fixed error handling in fetchFileFromIPFS.ts#223
Fixed error handling in fetchFileFromIPFS.ts#223PranjaliBhardwaj wants to merge 1 commit intoAOSSIE-Org:mainfrom
Conversation
Signed-off-by: Pranjali Bhardwaj <pranjalisharma6543@gmail.com>
📝 WalkthroughWalkthroughThe changes improve error handling in IPFS file fetching by adding explicit HTTP response validation and error propagation in the utility function, while the component adopts local error state management to capture and display these errors to users instead of silently failing. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/app/components/Fragment/CandidateDescription.tsx (1)
4-4: Use primitivestringinstead ofStringwrapper type.Same issue as in
fetchFileFromIPFS.ts— use the lowercase primitive type.Proposed fix
-const CandidateDescription = ({ IpfsHash }: { IpfsHash: String }) => { +const CandidateDescription = ({ IpfsHash }: { IpfsHash: string }) => {
🤖 Fix all issues with AI agents
In `@client/app/helpers/fetchFileFromIPFS.ts`:
- Line 9: The function fetchFileFromIPFS currently types its CID parameter as
the String object wrapper; change the parameter type to the primitive string
(i.e., export async function fetchFileFromIPFS(CID: string)) in the
fetchFileFromIPFS declaration and update any callers/type annotations that
reference this signature so they use the primitive string type as well.
🧹 Nitpick comments (2)
client/app/helpers/fetchFileFromIPFS.ts (1)
24-30: Error double-wrapping produces nested, redundant messages.When the
!response.okcheck throws at line 16, that error is immediately caught and re-wrapped at line 27, producing messages like:
"Error fetching IPFS file (CID: xyz): Failed to fetch file from IPFS: 404 Not Found"Consider re-throwing the original error if it already contains CID context, or only wrapping network/JSON-parse errors.
Proposed fix
} catch (error) { - // Re-throw the error with additional context if it's not already an Error - if (error instanceof Error) { - throw new Error(`Error fetching IPFS file (CID: ${CID}): ${error.message}`); + if (error instanceof Error && error.message.includes('IPFS')) { + // Already has IPFS context, re-throw as-is + throw error; } - throw new Error(`Unknown error fetching IPFS file (CID: ${CID})`); + const message = error instanceof Error ? error.message : 'Unknown error'; + throw new Error(`Error fetching IPFS file (CID: ${CID}): ${message}`); }client/app/components/Fragment/CandidateDescription.tsx (1)
24-28: The guard condition insideuseEffectis redundant.With an empty dependency array
[], this effect runs exactly once on mount. At that point,ipfsFile.nameis always""anderroris alwaysnull, so the guard always passes. The condition adds no protection and could confuse future readers.If the intent is a simple "fetch once on mount" pattern, remove the guard. If you need to prevent refetching on remounts or prop changes, consider adding
IpfsHashto the dependency array and using a ref oruseRefflag.Simplified version (fetch once on mount)
useEffect(() => { - if (ipfsFile.name === "" && !error) { - getIpfsFile(); - } + getIpfsFile(); }, []);
| * @returns Promise that resolves to the parsed JSON content of the file | ||
| * @throws Error if the fetch fails, response is not ok, or JSON parsing fails | ||
| */ | ||
| export async function fetchFileFromIPFS(CID: String) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use primitive string instead of String wrapper type.
TypeScript best practice is to use the lowercase primitive type string rather than the String object wrapper type. The String type refers to the JavaScript object wrapper, which is rarely what you want.
Proposed fix
-export async function fetchFileFromIPFS(CID: String) {
+export async function fetchFileFromIPFS(CID: string) {🤖 Prompt for AI Agents
In `@client/app/helpers/fetchFileFromIPFS.ts` at line 9, The function
fetchFileFromIPFS currently types its CID parameter as the String object
wrapper; change the parameter type to the primitive string (i.e., export async
function fetchFileFromIPFS(CID: string)) in the fetchFileFromIPFS declaration
and update any callers/type annotations that reference this signature so they
use the primitive string type as well.
1 participant
Description
Silent failures: the function returned undefined on error, making failures hard to detect.
No HTTP status validation: non-200 responses were treated as success.
Poor error context: errors lacked useful information.
Unused variable: removed unused JWT.
Fixes #222
How it was solved
Added HTTP status check:
if (!response.ok) { throw new Error(
Failed to fetch file from IPFS: ${response.status} ${response.statusText}); }Throws on non-2xx responses.
Improved error handling:
Re-throws errors with context (CID included).
Handles both Error instances and unknown errors.
Added JSDoc:
Documents parameters, return value, and thrown errors.
Updated caller (CandidateDescription.tsx):
Wraps the call in try/catch.
Shows a user-friendly error message.
Prevents crashes when IPFS fetch fails.
Code cleanup:
Removed unused JWT.
Removed console.log.
Type of change
Please mark the options that are relevant.
Checklist:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.