fix: serialize page_content to resolve mockserver create page error#1780
Conversation
… mockserver create page error
WalkthroughThe page service now includes a helper function to safely parse and normalize Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
hexqi
changed the title
page_content to JSON string to resolve…![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
mockServer/src/services/pages.js (2)
20-22: Don’t silently ignore JSON parse errors.Line 20-Line 22 suppresses parse failures completely. Keeping fallback behavior is fine, but add at least a warning log (with page id/route when available) to preserve debuggability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mockServer/src/services/pages.js` around lines 20 - 22, The catch block in mockServer/src/services/pages.js currently swallows JSON parse errors (the catch around the JSON parsing logic) — update it to log a warning instead of ignoring: in the catch for the JSON.parse/parse attempt inside the function handling page data (refer to the try/catch surrounding the parse in pages.js), call the logger (or console.warn if no logger exists) with a clear message that includes the page id/route variables available in that scope and the caught error object, then continue with the existing fallback behavior; ensure the log message includes both context (page id/route) and the error to aid debugging.
83-85: Consider explicit error handling forJSON.stringifyto improve observability.Lines 84 and 97 serialize request data without explicit error handling. While a global error middleware catches exceptions, adding try-catch at the point of serialization would improve code clarity and enable better error context (e.g., distinguishing serialization failures from other errors). Non-serializable payloads like circular references will propagate to clients as generic errors without this clarity.
Suggested improvement
+const serializePageContent = (value) => { + if (!value || typeof value !== 'object') { + return value + } + try { + return JSON.stringify(value) + } catch (error) { + error.message = `Invalid page_content: ${error.message}` + throw error + } +} + async create(params) { const model = params.isPage ? this.pageModel : this.folderModel const pageData = { ...model, ...params } - - if (pageData.page_content && typeof pageData.page_content === 'object') { - pageData.page_content = JSON.stringify(pageData.page_content) - } + pageData.page_content = serializePageContent(pageData.page_content) ... } async update(id, params) { const updateData = { ...params } - if (updateData.page_content && typeof updateData.page_content === 'object') { - updateData.page_content = JSON.stringify(updateData.page_content) - } + updateData.page_content = serializePageContent(updateData.page_content) ... }Also applies to: 96-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mockServer/src/services/pages.js` around lines 83 - 85, Wrap each JSON.stringify call in pages.js (the one serializing pageData.page_content and the other at lines around 96-98) in a try-catch: on failure catch the error, log it with contextual info (e.g., include pageData id or request body and the fact serialization failed), and then propagate a clear, specific error to upstream middleware (e.g., call next with a new Error/TypeError that includes the original error message or rethrow an error with that context) so serialization failures are distinguishable from other errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@mockServer/src/services/pages.js`:
- Around line 20-22: The catch block in mockServer/src/services/pages.js
currently swallows JSON parse errors (the catch around the JSON parsing logic) —
update it to log a warning instead of ignoring: in the catch for the
JSON.parse/parse attempt inside the function handling page data (refer to the
try/catch surrounding the parse in pages.js), call the logger (or console.warn
if no logger exists) with a clear message that includes the page id/route
variables available in that scope and the caught error object, then continue
with the existing fallback behavior; ensure the log message includes both
context (page id/route) and the error to aid debugging.
- Around line 83-85: Wrap each JSON.stringify call in pages.js (the one
serializing pageData.page_content and the other at lines around 96-98) in a
try-catch: on failure catch the error, log it with contextual info (e.g.,
include pageData id or request body and the fact serialization failed), and then
propagate a clear, specific error to upstream middleware (e.g., call next with a
new Error/TypeError that includes the original error message or rethrow an error
with that context) so serialization failures are distinguishable from other
errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 198ec8c1-8a42-4ee9-921b-865ca7f9c5cf
📒 Files selected for processing (1)
mockServer/src/services/pages.js
3 participants
English | 简体中文
PR
修复 mockServer 创建页面时因字段包含点号(.)导致报错的问题
背景与问题: 在调用 mockServer 的 /app-center/api/pages/create 接口创建页面时，若传入的 page_content（例如 css 的 class 名）包含 .（点号），底层依赖的 @seald-io/nedb 会因为 键名规范抛出 "Field names cannot contain a ." 错误，从而导致创建失败。
修复方案: 对mockserver pages相关接口page_content进行序列化
Create/Update：将对象的 page_content 字段通过 JSON.stringify 序列化为字符串
List/Detail：将 page_content 字符串通过 JSON.parse 重新反序列化为对象
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information