Skip to content

Conversation

@fayching
Copy link

@fayching fayching commented Oct 23, 2025

页面报错parseFunction error: {},原因是parseFunction函数传入的rawCode是"''",mock中的无用数据导致的

微信图片_20251022144259_89_10

English | 简体中文

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Built its own designer, fully self-validated

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Background and solution

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

  • Bug Fixes

    • Improved function parsing to safely handle empty or invalid function strings and consistently return null on errors.
  • Chores

    • Removed the obsolete "npm" utility component from application configuration and mock data sources.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 23, 2025

Walkthrough

Removed a non-functional "npm" utility component from three mock JSON files and hardened parseFunction to return null for empty/falsy code, evaluate code safely, and only bind when the evaluated result is a function; errors now consistently return null.

Changes

Cohort / File(s) Summary
Mock data npm component removal
mockServer/src/assets/json/appinfo.json, mockServer/src/mock/get/app-center/apps/extension/list.json, mockServer/src/mock/get/app-center/v1/apps/schema/1.json
Deleted the npm utility component/entry (id 146) which contained an empty JSFunction value ('') and minimal metadata from three mock data sources.
Function parsing logic hardening
packages/utils/src/utils/index.ts
parseFunction now returns null early for falsy or explicitly empty code strings ('' or ""); it constructs and calls Function('return (' + rawCode + ')') in the provided context, returns the result only if it's a function (bound to context), otherwise returns null; catch block now returns null on errors.

Sequence Diagram(s)

sequenceDiagram
  participant C as Caller
  participant P as parseFunction
  participant E as Evaluator

  C->>P: call parseFunction(rawCode, context)
  alt rawCode falsy or ""/''
    P-->>C: return null
  else
    P->>E: Function('return (' + rawCode + ')').call(context)
    alt evaluation throws
      E-->>P: throws
      P-->>C: return null
    else evaluation returns result
      E-->>P: result
      alt result is function
        P-->>C: return result.bind(context)
      else
        P-->>C: return null
      end
    end
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Points to check:

  • Verify parseFunction behavior with various inputs: valid function, non-function expression, empty strings, and malicious/unsafe code patterns.
  • Confirm mock JSON deletions do not break tests or fixtures that referenced id 146.

Poem

🐰
I pruned a ghost of npm away,
Empty code no longer gets to play,
Now functions bind only when they're true,
Errors return null — tidy and new,
Hop, patch, and onward we chew.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "fix:parseFunction error: failed to handle empty string input ''" directly and specifically describes the main fix implemented in the changeset. The primary code change in packages/utils/src/utils/index.ts explicitly adds early-return logic to handle empty strings and the "''" edge case, which aligns precisely with the title's description. The title is concise, specific, and clear enough for a developer scanning the history to understand that the PR addresses parseFunction's handling of empty string inputs. The JSON file removals appear to be cleanup of extraneous mock data that was contributing to the issue, which are secondary to the core fix.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 ddc4299 and 2105da0.

📒 Files selected for processing (4)
  • mockServer/src/assets/json/appinfo.json (0 hunks)
  • mockServer/src/mock/get/app-center/apps/extension/list.json (0 hunks)
  • mockServer/src/mock/get/app-center/v1/apps/schema/1.json (0 hunks)
  • packages/utils/src/utils/index.ts (1 hunks)
💤 Files with no reviewable changes (3)
  • mockServer/src/assets/json/appinfo.json
  • mockServer/src/mock/get/app-center/v1/apps/schema/1.json
  • mockServer/src/mock/get/app-center/apps/extension/list.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/utils/src/utils/index.ts

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.

@github-actions github-actions bot added the bug Something isn't working label Oct 23, 2025
Copy link
Contributor

@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)
packages/utils/src/utils/index.ts (1)

50-50: Consider improving error logging for better debugging.

JSON.stringify(error) typically returns "{}" for Error objects because their properties are non-enumerable, which explains the unhelpful error message mentioned in the PR description. While your defensive checks should prevent most errors from reaching this point, consider using more informative error logging.

Apply this diff to improve error logging:

-    console.error(`parseFunction error: ${JSON.stringify(error)}`)
+    console.error('parseFunction error:', error?.message || error)

Or for more detail:

-    console.error(`parseFunction error: ${JSON.stringify(error)}`)
+    console.error(`parseFunction error: ${error?.stack || error?.message || String(error)}`)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c27e843 and ddc4299.

📒 Files selected for processing (4)
  • mockServer/src/assets/json/appinfo.json (0 hunks)
  • mockServer/src/mock/get/app-center/apps/extension/list.json (0 hunks)
  • mockServer/src/mock/get/app-center/v1/apps/schema/1.json (0 hunks)
  • packages/utils/src/utils/index.ts (1 hunks)
💤 Files with no reviewable changes (3)
  • mockServer/src/mock/get/app-center/apps/extension/list.json
  • mockServer/src/mock/get/app-center/v1/apps/schema/1.json
  • mockServer/src/assets/json/appinfo.json
🔇 Additional comments (2)
packages/utils/src/utils/index.ts (2)

44-44: Good defensive check that fixes the root cause.

The early return correctly handles the case described in the PR where rawCode === "''" (a string literal containing two single quotes) would evaluate to an empty string when passed to the Function constructor, which would then fail when .bind() is called on the non-function result.

Note: The !rawCode check also catches all falsy values (0, false, null, undefined, empty string), which is appropriate for a code string parser.


46-47: Excellent defensive programming to prevent runtime errors.

The type check prevents calling .bind() on non-function results, which was the underlying cause of the error when rawCode evaluated to a string or other non-function value. This robustly handles cases where dynamic code evaluation produces unexpected types.

@fayching fayching changed the title fix:fix parseFunction error: {} fix:parseFunction error: failed to handle empty string input "''" Oct 23, 2025
@fayching fayching force-pushed the fayching/fix-parseFunction-error branch from ddc4299 to 2105da0 Compare October 30, 2025 06:43
Comment on lines +44 to +47
if (!rawCode || rawCode === "''" || rawCode === '""') return null
try {
return fun_ctor(`return (${rawCode})`).call(context).bind(context)
const result = fun_ctor(`return (${rawCode})`).call(context)
return typeof result === 'function' ? result.bind(context) : null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里可能不做拦截更好,直接进入 try catch,有错误可以直接控制台上看到错误,方便快速定位错误。
如果是希望不打印错误,或者是向上抛出错误,可以考虑增加配置/增加实现

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants