Skip to content

fix(url): resync cached searchParams when search/href changes - #422

Merged
NathanWalker merged 1 commit into
mainfrom
fix/url-searchparams-staleness
Jul 31, 2026
Merged

fix(url): resync cached searchParams when search/href changes#422
NathanWalker merged 1 commit into
mainfrom
fix/url-searchparams-staleness

Conversation

@edusperoni

@edusperoni edusperoni commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Fixes two defects in URL.prototype.searchParams reported by CodeRabbit on #411: #411 (comment)

The defects

  1. Staleness. The getter cached a URLSearchParams built from the query string on first access. A later direct assignment to url.search or url.href did not refresh it, so subsequent url.searchParams reads returned parameters from the old query. Worse, mutating through a reference held across such an assignment wrote the stale set back onto the URL, silently discarding the newly assigned query.
  2. Enumeration leak. The cache was stored as a plain enumerable own property, so _searchParams surfaced in Object.keys(url) and JSON.stringify(url).

Why resync in place instead of rebuilding

CodeRabbit's proposed diff rebuilds the cache into a new URLSearchParams when the recorded query no longer matches. That breaks the WHATWG same-object guarantee — url.searchParams must return the identical object for the URL's lifetime, and app code routinely holds a reference to it. A rebuild would leave every existing reference pointing at an object detached from its URL.

This PR keeps exactly one URLSearchParams per URL forever. When the recorded source string differs from the current this.search, the existing object is resynced in place: every key deleted, then re-appended from parsing the current query, then the recorded source updated. The resync goes through the raw methods captured at construction (_append, _delete, _forEach), never the public ones — those are wrapped to write back into _url.search and would clobber the very string being applied. The wrapped mutators also update the recorded source after writing back, so a mutation through the object never makes the next read look stale, and they resync first so a mutation on a reference held across a reassignment no longer clobbers the new query.

_searchParams and _searchParamsSource are defined non-enumerable via the destructured ObjectDefineProperty; all new intrinsic access goes through primordials (ArrayPrototypePush, FunctionPrototypeCall, both already captured — no additions to primordials.js or the eslint list were needed).

Tests

New TestRunner/app/tests/UrlSearchParamsTests.js (12 specs), registered in TestRunner/app/tests/index.js:

  • same-object identity across reads, and across a url.search = ... assignment
  • refresh after url.search assignment, after url.href assignment, and when the query is cleared
  • duplicate keys preserved across a refresh
  • mutation write-back (set/append/delete/sort update url.search / url.href)
  • mutation through the object does not trigger a resync that loses state on the next read
  • mutating a held reference after a query reassignment does not clobber the new query
  • Object.keys(url), JSON.stringify(url) and for..in do not expose _searchParams / _searchParamsSource
  • a fresh URL's first searchParams read parses correctly (including getAll and size)

Drive-by: ErrorEvents spec hardening

Three specs in ErrorEventsTests.js asserted that nothing at all reached the process-wide __onUncaughtError hook during their quiet window. That is inherently order-dependent — an async failure from an earlier suite (the deliberate module-not-found in NodeBuiltinsAndOptionalModulesTests.mjs) can drain into the spy while the test waits, and the extra specs added here shifted timing enough to make it land there. They now assert that the specific error under test did not reach the hook, which is what they actually mean.

Suite results

./run_tests.sh on the iOS simulator:

  • baseline (origin/main): 922 tests, 0 failures
  • this branch: 934 tests, 0 failures

ESLint (npx eslint 'NativeScript/runtime/js/**/*.js') clean; tools/js2c.mjs regen clean.

Summary by CodeRabbit

  • Bug Fixes

    • Improved URL.searchParams synchronization when URLs are updated through search or href.
    • Preserved stable parameter references while correctly reflecting query-string changes.
    • Ensured parameter mutations consistently update the associated URL.
  • Tests

    • Added comprehensive coverage for caching, duplicate keys, empty queries, refresh behavior, and mutation synchronization.
    • Refined error-event tests to verify that prevented errors are excluded from uncaught-error reports.

The URL.prototype.searchParams getter cached a URLSearchParams built
from the query string at first access. A later direct assignment to
url.search or url.href left the cache untouched, so subsequent reads
returned parameters from the old query, and mutating a held reference
wrote that stale set back onto the URL. The cache was also stored as a
plain enumerable own property, surfacing in Object.keys(url) and
JSON.stringify(url).

url.searchParams is a same-object accessor, so the object handed out for
a URL must stay identical for the URL's lifetime. Rather than rebuild
into a new instance when the query changes, the existing instance is
resynced in place — cleared and repopulated through the raw methods
captured at construction — and the query string it was built from is
recorded alongside it. Mutators resync before applying, so a mutation
through a reference held across a reassignment no longer clobbers the
new query. Both bookkeeping properties are non-enumerable.

The ErrorEvents specs asserted that nothing at all reached the
process-wide __onUncaughtError hook during their quiet window; they now
assert that the error under test did not, which is what they mean and is
immune to async failures draining in from earlier suites.
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The runtime now keeps URL.searchParams stable, synchronizes it after direct URL changes, and writes mutations back to the URL. Tests cover this behavior. Error event tests now check specific suppressed errors.

Changes

URLSearchParams synchronization

Layer / File(s) Summary
Cached search parameter synchronization
NativeScript/runtime/js/blob-url.js, TestRunner/app/tests/UrlSearchParamsTests.js, TestRunner/app/tests/index.js
The accessor preserves one parameter object, refreshes it after direct query changes, wraps mutations, and serializes updates back to the URL. Tests cover parsing, duplicates, held references, caching, synchronization, and enumeration.

Error event assertion updates

Layer / File(s) Summary
Specific uncaught-error assertions
TestRunner/app/tests/ErrorEventsTests.js
Suppression tests verify that their specific errors or rejection reasons are absent from uncaught-error reports.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant URL
  participant searchParams
  participant URLSearchParamsMethods

  URL->>searchParams: Access cached searchParams
  searchParams->>URLSearchParamsMethods: Resynchronize from URL.search
  URLSearchParamsMethods-->>searchParams: Update existing parameter object
  searchParams->>URL: Write serialized mutations to URL.search
Loading

Suggested reviewers: nathanwalker

Poem

A rabbit hops through query strings bright,
Keeps cached params tidy and light.
Direct changes flow, mutations return,
Tests watch each detail they discern.
Suppressed errors stay out of sight.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main fix: resynchronizing cached URL.searchParams after search or href changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@NativeScript/runtime/js/blob-url.js`:
- Around line 100-124: Update the raw method caches in the searchParams setup to
define _forEach, _append, _delete, _set, and _sort via ObjectDefineProperty with
enumerable set to false while preserving their writable callable values. Extend
the cache test using _searchParams and _searchParamsSource to verify these
internal properties are absent from Object.keys and for...in enumeration.
- Around line 79-128: Update the searchParams initialization logic in the
URL.prototype.searchParams getter so read methods—get, getAll, has, keys,
values, entries, forEach, toString, size, and Symbol.iterator—resync against the
owning URL before returning data. Preserve the existing original-method
delegation pattern used by append, delete, set, and sort, or apply an equivalent
proxy covering every listed access point.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 92c8aca4-3559-4f72-bf56-7375edb6045a

📥 Commits

Reviewing files that changed from the base of the PR and between 5e40702 and e6ecf57.

📒 Files selected for processing (4)
  • NativeScript/runtime/js/blob-url.js
  • TestRunner/app/tests/ErrorEventsTests.js
  • TestRunner/app/tests/UrlSearchParamsTests.js
  • TestRunner/app/tests/index.js

Comment on lines 79 to 128
ObjectDefineProperty(URL.prototype, 'searchParams', {
get() {
if (this._searchParams == null) {
this._searchParams = new URLSearchParamsCtor(this.search);
ObjectDefineProperty(this._searchParams, '_url', {
const params = new URLSearchParamsCtor(this.search);
ObjectDefineProperty(this, '_searchParams', {
enumerable: false,
writable: true,
configurable: true,
value: params,
});
ObjectDefineProperty(this, '_searchParamsSource', {
enumerable: false,
writable: true,
configurable: true,
value: this.search,
});
ObjectDefineProperty(params, '_url', {
enumerable: false,
writable: false,
value: this,
});
this._searchParams._append = this._searchParams.append;
this._searchParams.append = function (name, value) {
params._forEach = params.forEach;
params._append = params.append;
params.append = function (name, value) {
resync(this);
this._append(name, value);
this._url.search = this.toString();
writeBack(this);
};
this._searchParams._delete = this._searchParams.delete;
this._searchParams.delete = function (name) {
params._delete = params.delete;
params.delete = function (name) {
resync(this);
this._delete(name);
this._url.search = this.toString();
writeBack(this);
};
this._searchParams._set = this._searchParams.set;
this._searchParams.set = function (name, value) {
params._set = params.set;
params.set = function (name, value) {
resync(this);
this._set(name, value);
this._url.search = this.toString();
writeBack(this);
};
this._searchParams._sort = this._searchParams.sort;
this._searchParams.sort = function () {
params._sort = params.sort;
params.sort = function () {
resync(this);
this._sort();
this._url.search = this.toString();
writeBack(this);
};
} else {
resync(this._searchParams);
}
return this._searchParams;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== repo files matching blob-url/tests =="
git ls-files | rg '(^|/)blob-url\.js$|UrlSearchParamsTests\.js$|blob|searchParams|URLSearchParams' || true

echo
echo "== blob-url outline/size =="
if [ -f NativeScript/runtime/js/blob-url.js ]; then
  wc -l NativeScript/runtime/js/blob-url.js
  ast-grep outline NativeScript/runtime/js/blob-url.js --view compact || true
  echo "== relevant blob-url lines =="
  sed -n '1,180p' NativeScript/runtime/js/blob-url.js | cat -n
fi

echo
echo "== UrlSearchParamsTests relevant sections =="
if [ -f NativeScript/runtime/js/UrlSearchParamsTests.js ]; then
  wc -l NativeScript/runtime/js/UrlSearchParamsTests.js
  git diff -- NativeScript/runtime/js/UrlSearchParamsTests.js | sed -n '1,240p'
  rg -n "searchParams|search|href|append|get\\(|getAll\\(|has\\(|keys\\(|values\\(|entries\\(|forEach\\(|toString\\(|sort\\(|Symbol|iterator|size" NativeScript/runtime/js/UrlSearchParamsTests.js NativeScript/runtime/js/blob-url.js || true
fi

Repository: NativeScript/ios

Length of output: 6055


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== test file relevant lines =="
wc -l TestRunner/app/tests/UrlSearchParamsTests.js
sed -n '1,260p' TestRunner/app/tests/UrlSearchParamsTests.js | cat -n

echo
echo "== URLSearchParams.js test list around searchParams =="
awk '
/URLSearchParams|searchParams|function \(title\)/{start=NR}
NR>=start && NR<=start+220 || NR>=1 && NR<=220 {print NR": "$0}
' TestRunner/app/tests/URLSearchParams.js | cat -n

echo
echo "== static probe for wrapped and unwrapped URLSearchParams methods in blob-url.js =="
python3 - <<'PY'
from pathlib import Path
p=Path("NativeScript/runtime/js/blob-url.js")
s=p.read_text()
start=s.index("ObjectDefineProperty(URL.prototype, 'searchParams', {")
wrap=s.index("params.append = function", start)
end=None
try:
    end=s.index("});\n);", wrap)
except ValueError:
    end=s.find("});", wrap + 1)
if wrap==-1:
    raise SystemExit("wrapper section not found")
section=s[start:end]
unwrapped_methods="get getAll has keys values entries forEach toString size".split()
for name in unwrapped_methods:
    needle=f"params.{name}"
    if name != "size":
        needle=f"{name}:"
    print(f"{name}: wrapper_assign={'params._'+name in section and 'params.'+name+' = function' in section} wrapper_calls_resync={'params._'+name+" in section and "resync(this)\n                return this._"+name in section)
print("size" in section)
PY

Repository: NativeScript/ios

Length of output: 35078


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate and inspect URLSearchParams implementation =="
fd -a 'URLSearchParamsImpl\.(h|cpp)$' . | while read -r f; do
  echo "--- $f ---"
  wc -l "$f"
  sed -n '1,260p' "$f" | cat -n
done

echo
echo "== check whether searchParams tests cover direct method stale path =="
python3 - <<'PY'
from pathlib import Path
p = Path("TestRunner/app/tests/UrlSearchParamsTests.js")
s = p.read_text()
cases = s.split("it(")
held = []
stale_read_after_url_change = []
for i, case in enumerate(cases[1:], 1):
    # crude brace/expect boundary
    code = case[:case.find(";)")]
    if "url.searchParams" in code and "url.search" in code:
        all_sp = code.count("url.searchParams")
    else:
        all_sp = 0
    if "const sp = url.searchParams" in code and "url.search = " in code:
        held.append((i, "has held sp after search reassign"))
        # Check direct read methods on sp after url.search reassign without reading url.searchParams immediately.
        after_assign = code.index("url.search = ")
        segment = code[after_assign:]
        direct_sp_reads = ["sp get", "sp getAll", "sp has", "sp keys", "sp values", "sp entries", "sp forEach", "sp toString", "sp [Symbol", "sp.iterator", "sp size"]
        if "sp.set" in segment or "sp.append" in segment or "sp.delete" in segment or "sp.sort" in segment:
            stale_read_after_url_change.append((i, "mutator on sp before direct read"))
        else:
            found = []
            for term in direct_sp_reads:
                if term in segment:
                    found.append(term)
            stale_read_after_url_change.append((i, f"has held sp after search reassign; direct reads not present: {found or 'none'}"))
print("held-after-search tests:", len(held))
print("held-after-search expectations involving sp (not url.searchParams):")
for idx, text in held:
    print(f"  {idx}: {text}")
print("held-after-search stale direct-read status:")
for idx, text in stale_read_after_url_change:
    print(f"  {idx}: {text}")
PY

echo
echo "== deterministic JS semantics probe for current wrapper coverage =="
node - <<'JS'
const methods = ["get","getAll","has","keys","values","entries","forEach","toString","size"];
const source = require('fs').readFileSync('NativeScript/runtime/js/blob-url.js','utf8');
const section = source.substring(600); // after relevant constants
for (const name of methods) {
  const pattern = new RegExp(`params\\.${name}\\s*=\\s*function`)
  console.log(name, "has wrapper", pattern.test(source), section.includes(`resync(this)`), section.includes(`return this._${name}`), section.includes(`${name}:`));
}
JS

Repository: NativeScript/ios

Length of output: 16590


Make searchParams read methods resync before returning data.

get, getAll, has, keys, values, entries, forEach, toString, size, and Symbol.iterator are currently read directly. Keep a held url.searchParams reference, assign a new search/hash, and call one of these methods; it can return the old query state because only append, delete, set, sort, and the searchParams getter itself resync. Wrap these access points with the same resync(this)/original-delegate pattern, or use a Proxy that resyncs on every access.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@NativeScript/runtime/js/blob-url.js` around lines 79 - 128, Update the
searchParams initialization logic in the URL.prototype.searchParams getter so
read methods—get, getAll, has, keys, values, entries, forEach, toString, size,
and Symbol.iterator—resync against the owning URL before returning data.
Preserve the existing original-method delegation pattern used by append, delete,
set, and sort, or apply an equivalent proxy covering every listed access point.

Comment on lines +100 to 124
params._forEach = params.forEach;
params._append = params.append;
params.append = function (name, value) {
resync(this);
this._append(name, value);
this._url.search = this.toString();
writeBack(this);
};
this._searchParams._delete = this._searchParams.delete;
this._searchParams.delete = function (name) {
params._delete = params.delete;
params.delete = function (name) {
resync(this);
this._delete(name);
this._url.search = this.toString();
writeBack(this);
};
this._searchParams._set = this._searchParams.set;
this._searchParams.set = function (name, value) {
params._set = params.set;
params.set = function (name, value) {
resync(this);
this._set(name, value);
this._url.search = this.toString();
writeBack(this);
};
this._searchParams._sort = this._searchParams.sort;
this._searchParams.sort = function () {
params._sort = params.sort;
params.sort = function () {
resync(this);
this._sort();
this._url.search = this.toString();
writeBack(this);
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate file =="
fd -a 'blob-url\.js$|UrlSearchParamsTests\.js$' . || true

echo "== git status/stat =="
git diff --stat || true

echo "== blob-url lines 1-160 =="
blob=$(fd 'blob-url\.js$' . | head -n1)
[ -n "${blob:-}" ] && cat -n "$blob" | sed -n '1,180p'

echo "== tests around 130-170 =="
testfile=$(fd 'UrlSearchParamsTests\.js$' . | head -n1)
[ -n "${testfile:-}" ] && cat -n "$testfile" | sed -n '110,170p'

echo "== search ObjectDefineProperty and own keys tests =="
rg -n "ObjectDefineProperty|Object\.keys|OwnPropertyKeys|for\.\\(\\w+ in|ownKeys|searchParams|_url|_searchParams|_searchParamsSource" -S .

Repository: NativeScript/ios

Length of output: 50372


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Object.defineProperty enumerable probe =="
node - <<'JS'
const obj = {};
obj._raw = Object.prototype.forEach.call;
Reflect.defineProperty(obj, '_bound', { enumerable: false, writable: true, configurable: true, value: Object.prototype.forEach.call });

console.log(objectKeysContains, forInContains);

function objectKeysContains(prop) {
  return Object.keys(obj).includes(prop);
}

function forInContains(prop) {
  let found = false;
  for (const key in obj) {
    if (key === prop) found = true;
  }
  return found;
}

console.log("_raw in Object.keys:", objectKeysContains("_raw"));
console.log("_raw in for...in:", forInContains("_raw"));
console.log("_bound in Object.keys:", objectKeysContains("_bound"));
console.log("_bound in for...in:", forInContains("_bound"));
console.log("descriptor:", Object.getOwnPropertyDescriptor(obj, "_bound"));

const keys = Object.keys(obj);
for (const key in obj) {
  console.log("enumerable key:", key);
}
console.log("has own non-enumerable:", Object.prototype.hasOwnProperty.call(obj, "_bound"));
console.log("hasOwnProperty on enumerable cache key:", Object.prototype.hasOwnProperty.call(obj, "_raw"));
JS

echo "== focused tests =="
cat -n TestRunner/app/tests/UrlSearchParamsTests.js | sed -n '1,220p'

Repository: NativeScript/ios

Length of output: 860


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Object.prototype assignment/defineProperty enumerable probe =="
node - <<'JS'
const obj = {};
obj._raw = Function.prototype.call.bind(Object.prototype.forEach);

Reflect.defineProperty(obj, '_bound', { enumerable: false, writable: true, configurable: true, value: Function.prototype.call.bind(Object.prototype.forEach) });

function objectKeysContains(prop) {
  return Object.keys(obj).includes(prop);
}

function forInContains(prop) {
  let found = false;
  for (const key in obj) {
    if (key === prop) found = true;
  }
  return found;
}

console.log("_raw in Object.keys:", objectKeysContains("_raw"));
console.log("_raw in for...in:", forInContains("_raw"));
console.log("_bound in Object.keys:", objectKeysContains("_bound"));
console.log("_bound in for...in:", forInContains("_bound"));
console.log("descriptor:", Object.getOwnPropertyDescriptor(obj, "_bound"));

const keys = Object.keys(obj);
for (const key in obj) {
  console.log("enumerable key:", key);
}
console.log("has own enumerable cache property:", Object.prototype.hasOwnProperty.call(obj, "_raw"));
JS

echo "== focused UrlSearchParamsTests =="
cat -n TestRunner/app/tests/UrlSearchParamsTests.js | sed -n '1,220p'

Repository: NativeScript/ios

Length of output: 6127


Make the params raw method caches non-enumerable.

obj._for = obj.forEach; on url.searchParams creates a writable, enumerable own property, so Object.keys(url.searchParams) and for...in expose _forEach, _append, _delete, _set, and _sort. Use ObjectDefineProperty(params, '_<name>', { enumerable: false, ... }) for each cached method, and cover searchParams enumeration in the cache test with _searchParams/_searchParamsSource.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@NativeScript/runtime/js/blob-url.js` around lines 100 - 124, Update the raw
method caches in the searchParams setup to define _forEach, _append, _delete,
_set, and _sort via ObjectDefineProperty with enumerable set to false while
preserving their writable callable values. Extend the cache test using
_searchParams and _searchParamsSource to verify these internal properties are
absent from Object.keys and for...in enumeration.

@NathanWalker

Copy link
Copy Markdown
Contributor

@coderabbitai could you investigate the failing ApiTests in the CI test run?

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="NativeScript/ios"
pr_number="422"

echo "== PR checks =="
gh pr checks "$pr_number" --repo "$repo" --required || true

echo
echo "== Check runs matching ApiTests =="
gh api "repos/$repo/commits/$(gh pr view "$pr_number" --repo "$repo" --json headRefOid --jq .headRefOid)/check-runs?per_page=100" \
  --jq '.check_runs[] | select(.name | test("ApiTests"; "i")) | {id, name, status, conclusion, details_url, started_at, completed_at, output}' || true

echo
echo "== Recent workflow runs for PR =="
gh run list --repo "$repo" --branch "$(gh pr view "$pr_number" --repo "$repo" --json headRefName --jq .headRefName)" --limit 20 \
  --json databaseId,name,status,conclusion,url,headSha,createdAt,updatedAt

Length of output: 1637


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="NativeScript/ios"
run_id="30608513718"

echo "== Jobs in failed workflow run =="
gh run view "$run_id" --repo "$repo" --json jobs \
  --jq '.jobs[] | {databaseId, name, status, conclusion, startedAt, completedAt, steps}'

echo
echo "== Failed-step log output =="
gh run view "$run_id" --repo "$repo" --log-failed || true

echo
echo "== Workflow definition =="
workflow_path=$(gh api "repos/$repo/actions/runs/$run_id" --jq .path)
echo "$workflow_path"
gh api "repos/$repo/contents/$workflow_path?ref=e6ecf57aa0ad55157984a0953ff62960f995a7c6" --jq .content | base64 --decode | sed -n '1,260p'

Length of output: 198870


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="NativeScript/ios"
test_file=$(fd -a '^ApiTests\.js$' TestRunner | head -n 1)

echo "== ApiTests.js structure =="
ast-grep outline "$test_file" --items all --view expanded

echo
echo "== NSURLSession test implementation and setup =="
rg -n -C 24 'NSURLSession|downloadTaskWithURLCompletionHandler' "$test_file"

echo
echo "== Recent completed pull-request workflow runs on main =="
gh run list --repo "$repo" --workflow pull_request.yml --branch main --status completed --limit 12 \
  --json databaseId,conclusion,headSha,createdAt,url

echo
echo "== Same failure in recent failed main workflow logs =="
for run_id in $(gh run list --repo "$repo" --workflow pull_request.yml --branch main --status completed --limit 12 --json databaseId,conclusion --jq '.[] | select(.conclusion == "failure") | .databaseId'); do
  echo "--- Run $run_id ---"
  gh run view "$run_id" --repo "$repo" --log-failed 2>/dev/null \
    | rg -n -C 3 "NSURLSession\.sharedSession\.downloadTaskWithURLCompletionHandler|downloadTaskWithURLCompletionHandler's|FAIL.*ApiTests" \
    || true
done

Length of output: 4136


@NathanWalker Investigation complete.

The CI failure is unrelated to this PR. All URL and URLSearchParams tests passed.

The only failed test is in TestRunner/app/tests/ApiTests.js:

NSURLSession.sharedSession.downloadTaskWithURLCompletionHandler's

The test downloads this external HTTP resource:

http://upload.wikimedia.org/wikipedia/commons/7/7f/Williams_River-27527.jpg

The test failed after 5.347 seconds and reported only [object Object]. This duration is consistent with the test timeout. The test calls done only from the successful completion handler. It has no error-path handling. If the request stalls, redirects unsuccessfully, or cannot reach Wikimedia, the test times out with an unhelpful error.

The test is therefore network-dependent and flaky. It is not caused by the URL.prototype.searchParams cache changes.

Recommended follow-up:

  1. Replace the live Wikimedia request with a deterministic local test server or fixture.
  2. Until then, add completion-handler error handling so CI reports the underlying NSError.
  3. If a live request must remain, use HTTPS and a timeout that matches the CI network conditions.

You are interacting with an AI system.

@NathanWalker
NathanWalker merged commit 10547cc into main Jul 31, 2026
8 of 9 checks passed
@NathanWalker
NathanWalker deleted the fix/url-searchparams-staleness branch July 31, 2026 17:20
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.

2 participants