Skip to content

Avoid crash after mapValueError()#1680

Open
coreh wants to merge 2 commits intoelysiajs:mainfrom
coreh:fix-error-summary-crash
Open

Avoid crash after mapValueError()#1680
coreh wants to merge 2 commits intoelysiajs:mainfrom
coreh:fix-error-summary-crash

Conversation

@coreh
Copy link

@coreh coreh commented Jan 17, 2026

Hit this error when upgrading to Elysia 1.4.22:

Error: undefined is not an object (evaluating 'mapValueError(error).summary')

mapValueError() returns early if error is undefined.

The code above the change line correctly uses ?. for guarding against that, but this line doesn't.

Summary by CodeRabbit

  • Bug Fixes
    • Improved error-handling reliability during validation to prevent potential runtime errors when a validation summary is missing, ensuring more robust and stable behavior for edge cases.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 17, 2026

Walkthrough

Optional chaining was added when accessing summary from the result of mapValueError() in error handling, avoiding a potential runtime error if mapValueError() returns undefined. No other logic changes were introduced.

Changes

Cohort / File(s) Summary
Error handling safety improvement
src/error.ts
Use optional chaining (mapValueError(error)?.summary) in ValidationError construction at two locations to avoid accessing summary on undefined

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

I’m a careful rabbit, light on my feet,
I check each branch so none skip a beat.
With a tiny question mark I softly pry,
No undefined shadows passing by.
Hooray for safety — a small, happy sigh! 🐰✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding optional chaining to prevent crashes when mapValueError() returns undefined.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/error.ts (1)

418-437: Guard the remaining mapValueError(error).summary access.

This fix is partial: the non-customError JSON path still calls mapValueError(error).summary (Line 461), which can crash when error is undefined. Please apply the same optional chaining there to fully close the hole.

🔧 Proposed fix
-						summary: mapValueError(error).summary,
+						summary: mapValueError(error)?.summary,

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant