-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: add invalid notion document #1658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughExplicit error handling was added to both the Notion upload process in the document modal and the document creation utility. Now, when the API response is not successful, the error message is extracted from the response JSON and surfaced to the user, preventing further processing and unhandled exceptions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AddDocumentModal
participant NotionAPI
participant Toast
User->>AddDocumentModal: Initiate Notion page upload
AddDocumentModal->>NotionAPI: POST Notion page
NotionAPI-->>AddDocumentModal: Response (OK or Error)
alt Response not OK
AddDocumentModal->>NotionAPI: Parse error message from JSON
AddDocumentModal->>Toast: Show error message
AddDocumentModal-->>User: Stop further processing
else Response OK
AddDocumentModal->>AddDocumentModal: Continue with document logic
end
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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
🧹 Nitpick comments (3)
lib/documents/create-document.ts (2)
88-90: Inconsistent error handling across functions.This function uses a different error handling approach than the updated one above (line 51-54). For consistency, consider updating this error handler to match the improved approach used in
createDocument.if (!response.ok) { - throw new Error(`HTTP error! status: ${response.status}`); + const error = await response.json(); + throw new Error(typeof error === 'string' ? error : error.message || JSON.stringify(error)); }
125-127: Inconsistent error handling across functions.This function also uses a different error handling approach than the updated one in
createDocument. For consistency, consider updating this error handler as well.if (!response.ok) { - throw new Error(`HTTP error! status: ${response.status}`); + const error = await response.json(); + throw new Error(typeof error === 'string' ? error : error.message || JSON.stringify(error)); }components/documents/add-document-modal.tsx (1)
365-433: Catch potential JSON parsing errors.If the response isn't valid JSON, the current implementation might throw uncaught exceptions. Consider adding a try-catch block around JSON parsing to handle potential errors more gracefully.
Wrap the response parsing in try-catch:
try { setUploading(true); const response = await fetch( `/api/teams/${teamInfo?.currentTeam?.id}/documents`, { method: "POST", headers: { "Content-Type": "application/json", }, body: JSON.stringify({ name: createNotionFileName(), url: notionLink, numPages: 1, type: "notion", createLink: false, folderPathName: currentFolderPath?.join("/"), }), }, ); + let responseData; + try { if (!response.ok) { - const { error } = await response.json(); + responseData = await response.json(); + const { error } = responseData; toast.error(error); return; } - const document = await response.json(); + responseData = responseData || await response.json(); + const document = responseData; + } catch (parseError) { + toast.error("Failed to parse server response"); + console.error("JSON parsing error:", parseError); + return; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
components/documents/add-document-modal.tsx(1 hunks)lib/documents/create-document.ts(1 hunks)
🔇 Additional comments (2)
components/documents/add-document-modal.tsx (2)
366-370: Good error handling improvement for Notion uploads.Adding explicit error handling for failed Notion page uploads is a great improvement, ensuring users receive helpful error messages rather than unexpected failures.
371-372: 🛠️ Refactor suggestionPotential redundant JSON parsing.
Since the response body stream can only be consumed once, this second call to
response.json()might fail if the error branch was executed previously. Consider storing the parsed result after the first check.if (!response.ok) { const { error } = await response.json(); toast.error(error); return; } -const document = await response.json(); +const responseData = await response.json(); +const document = responseData;Likely an incorrect or invalid review comment.
| if (!response.ok) { | ||
| throw new Error(`HTTP error! status: ${response.status}`); | ||
| const error = await response.json(); | ||
| throw new Error(error); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improved error handling, but consider refining further.
The change enhances error handling by extracting error details from the response JSON, but there are a couple of concerns:
- Passing an object directly to the Error constructor might result in "[object Object]" as the error message.
- No error handling if the response body isn't valid JSON.
if (!response.ok) {
const error = await response.json();
- throw new Error(error);
+ throw new Error(typeof error === 'string' ? error : error.message || JSON.stringify(error));
}🤖 Prompt for AI Agents
In lib/documents/create-document.ts around lines 51 to 54, the error handling
currently throws a new Error with the entire error object, which results in an
unhelpful "[object Object]" message and lacks handling for invalid JSON
responses. To fix this, parse the response JSON inside a try-catch block,
extract a meaningful error message string from the parsed object, and throw a
new Error with that message. If parsing fails, throw a generic error indicating
the response could not be parsed.
Summary by CodeRabbit