fix(worker): throw when an environment-data key cannot be converted to a string by adrian-niculescu · Pull Request #2066 · NativeScript/android · GitHub

setEnvironmentData(key, value) with a key whose toString() throws does not throw. It stores the value under the empty-string key instead, replacing whatever was there, and getEnvironmentData(key) and setEnvironmentData(key) read or delete that unrelated entry.

Both callbacks convert the key with a helper that swallows a throwing toString() and returns an empty string. They now convert it with a checked ToString and return with the exception pending, so it reaches the caller. Keys stay stringified, as the shared stringifies keys spec expects. A symbol key now throws a TypeError, as any string conversion of a symbol does, rather than mapping to the empty-string key.

Stacked on #2043; the same fix for iOS is NativeScript/ios#492. The throwing-key spec fails on that branch and passes here, a second spec keeps a key with an embedded NUL distinct from its prefix, and the full device suite passes.

Summary by CodeRabbit

Bug Fixes
- Environment-data keys containing null characters are now kept distinct from their prefixes, and deleting a key no longer affects its prefix.
- Errors raised while converting keys to strings now propagate correctly when setting or retrieving environment data.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 680df5d4-5369-4648-9bd4-2df5f8d29cc2

📥 Commits

Reviewing files that changed from the base of the PR and between 42a8bcf and 1e23b14.

📒 Files selected for processing (2)

test-app/app/src/main/assets/app/tests/testMessaging.js
test-app/runtime/src/main/cpp/Messaging.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Walkthrough

Walkthrough

Environment-data callbacks now convert keys with V8 in the current context and stop if conversion fails. Regression tests cover NUL-containing keys, deletion, and exceptions thrown during key conversion.

Changes

Environment-data key handling

Layer / File(s)	Summary
Key conversion and regression coverage test-app/runtime/src/main/cpp/Messaging.cpp, test-app/app/src/main/assets/app/tests/testMessaging.js	The set and get callbacks use V8 key conversion and return when conversion fails. Tests cover keys containing NUL, deletion, and exceptions from toString.

Priority: ⚪ Not assessed

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 1e23b

Failed key conversions now reach callers instead of accessing the empty-string entry, and the reviewed changes show no material merge risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: environment-data key conversion errors now propagate to the caller.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1 🛠️ Fix failing CI checks 💡

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

A rabbit taps a key with care
V8 converts it in its lair
NUL keys keep their values true
Exceptions pass right through
The prefix stays, the tests are done
The rabbit hops beneath the sun

Comment @coderabbitai help to get the list of available commands.

…o a string setEnvironmentData and getEnvironmentData converted the key with a helper that swallows a throwing toString() and returns an empty string, so such a key silently read, overwrote or deleted the unrelated empty-string entry. The conversion is checked now and the exception reaches the caller.

@coderabbitai review

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

adrian-niculescu mentioned this pull request Oct 6, 2026

fix(worker): throw when an environment-data key cannot be converted to a string NativeScript/ios#492

Open

adrian-niculescu force-pushed the fix/environment-data-key-conversion branch from c38bdae to 8adf08f Compare October 6, 2026 18:02

adrian-niculescu marked this pull request as ready for review October 6, 2026 19:19

adrian-niculescu force-pushed the fix/environment-data-key-conversion branch from 8adf08f to 1e23b14 Compare October 6, 2026 19:24

adrian-niculescu mentioned this pull request Oct 6, 2026

feat: MessagePort, MessageChannel, BroadcastChannel, and node:worker_threads #2043

Open

adrian-niculescu mentioned this pull request Oct 6, 2026

fix(runtime): deliver a posted undefined as undefined, not null NativeScript/ios#477

Merged

fix(worker): throw when an environment-data key cannot be converted to a string by adrian-niculescu · Pull Request #2066 · NativeScript/android · GitHub

Repository navigation

fix(worker): throw when an environment-data key cannot be converted to a string - #2066

fix(worker): throw when an environment-data key cannot be converted to a string#2066
adrian-niculescu wants to merge 1 commit into
NativeScript:feat/worker-threadsfrom
adrian-niculescu:fix/environment-data-key-conversion

adrian-niculescu commented Oct 6, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 6, 2026 •
edited
Loading

Uh oh!

Uh oh!

adrian-niculescu commented Oct 6, 2026

Uh oh!

coderabbitai Bot commented Oct 6, 2026 •
edited
Loading

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

fix(worker): throw when an environment-data key cannot be converted to a string by adrian-niculescu · Pull Request #2066 · NativeScript/android · GitHub

Repository navigation

fix(worker): throw when an environment-data key cannot be converted to a string - #2066

Conversation

adrian-niculescu commented Oct 6, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

adrian-niculescu commented Oct 6, 2026

Uh oh!

coderabbitai Bot commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

adrian-niculescu commented Oct 6, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Oct 6, 2026 •
edited
Loading

coderabbitai Bot commented Oct 6, 2026 •
edited
Loading