src: remove ToLocalChecked() from UnionBytes

[iknoom]

This PR removes ToLocalChecked() usage from UnionBytes to avoid unexpected crashes.

• Rename UnionBytes::ToStringChecked() -> UnionBytes::ToString().
  • Remove ToLocalChecked() usage.
  • Change return type to MaybeLocal.
• Update call sites to handle empty MaybeLocal.

[nodejs-github-bot]

Review requested:

• @nodejs/startup

[codecov]

Codecov Report

❌ Patch coverage is 58.33333% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.54%. Comparing base (b13f24c) to head (59aef2d).
⚠️ Report is 21 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_builtins.cc	44.44%	2 Missing and 3 partials ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main   #60164    +/-   ##
========================================
  Coverage   88.53%   88.54%            
========================================
  Files         703      704     +1     
  Lines      207997   208094    +97     
  Branches    40015    40024     +9     
========================================
+ Hits       184150   184251   +101     
+ Misses      15864    15858     -6     
- Partials     7983     7985     +2     

Files with missing lines	Coverage Δ
src/node_union_bytes.h	100.00% <ø> (ø)
src/util.cc	85.62% <100.00%> (-0.06%)	⬇️
src/node_builtins.cc	78.95% <44.44%> (-0.37%)	⬇️

... and 26 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[legendecas]

UnionBytes is used for built-in strings (like built-in module sources). It is intended that if a UnionBytes failed to create the external string, the process should crash as soon as possible.

[addaleax]

This change kind of only brings us halfway towards the right semantics, because the comment in this snippet also applies here:

node/src/util-inl.h

Lines 347 to 357 in 1527825

	if (str.size() >= static_cast<size_t>(v8::String::kMaxLength)) [[unlikely]] {
	// V8 only has a TODO comment about adding an exception when the maximum
	// string size is exceeded.
	ThrowErrStringTooLong(isolate);
	return v8::MaybeLocal<v8::Value>();
	}
	return v8::String::NewFromUtf8(
	isolate, str.data(), v8::NewStringType::kNormal, str.size())
	.FromMaybe(v8::Local<v8::String>());
	}

[addaleax]

Just a suggestion, but if we add support for UnionBytes in ToV8Value(), this entirely loop could be replaced by a ToV8Value() call

[addaleax]

Let's not do this – this is dangerous and strictly worse than the previous code, because a return type of Local<String> (as opposed to MaybeLocal<String>) typically means that the value won't be empty, so the caller might assume that it's always safe to dereference the handle.

[iknoom]

Thanks for the review and guidance! I approached this too narrowly without fully understanding the overall design. I’ll close this PR.