Skip to content

Leave the Cloud Request Engine's data out of the JSON Builder's output - #91

Merged
jwrosewell merged 3 commits into
rcfrom
fix/json-builder-leaves-out-cloud
Nov 12, 2026
Merged

jwrosewell merged 3 commits into
rcfrom
fix/json-builder-leaves-out-cloud

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Problem

The JSON Builder writes the Cloud Request Engine's Element Data into the JSON it builds for the browser. That Element Data is the cloud service's whole answer, held as one string under the data key cloud (cloudrequestengine.py lines 121 to 128 and 490 on rc at 126d3733). JSONBundlerElement.process_internal leaves out only jsonbundler, javascriptbuilder and set-headers (jsonbundler.py lines 126 to 129 on rc at 126d3733), so the JSON sent to the browser carries the raw answer under cloud.

What the specification says

json-builder.md lines 80 to 92, specifications at 6570727d, in the Process section:

Some Elements MUST be excluded from having their Properties added to the JSON.
These are:

The reference implementations achieve this with a hard-coded exclusion list which
contains the Data Key of the elements to exclude, being set-headers,
cloud-response and json-builder. The usage sharing element adds no Element
Data to the Flow Data, so it needs no entry in that list.

Python's Cloud Request Engine uses the data key cloud where the reference implementations use cloud-response, so cloud is the entry Python's list needs.

The .NET, Java, Go, Node.js and PHP packages already leave the Cloud Request Engine out.

Language Entry Where, on each repository's rc
.NET cloud-response JsonBuilderElement.cs line 174 at de7d1922
Java cloud-response JsonBuilderElement.java line 102 at 0b9257f4
Go cloud-response jsonbuilder.go line 57 at 37c4592f
Node.js cloud jsonbundler.js line 36 at 871e7a37
PHP cloud JsonBundlerElement.php line 50 at 5be49565

What changed

jsonbundler.py adds cloud to the exclusion list (lines 126 to 130 at the head). A code search for datakey = "cloud in this repository finds only cloudrequestengine.py and cloudengine.py, whose base data key is CloudEngineBase, so no other element drops out of the JSON.

The current rc (126d373) is merged into the branch at fb22e7b with GitHub's update branch, and the merge was clean. Since this draft was opened, rc took #70 (3c786b73), which removed sequence from the same list so that the Sequence Element's data goes into the JSON. The two edits touch different lines, and the merged list keeps both, with sequence gone and cloud added. The whole difference from rc is jsonbundler.py and the new test.

How it was verified

New test test_jsonbundler_excludes.py, with a stand-in for the Cloud Request Engine that writes the raw answer under cloud with the same property the real engine declares.

  • Red, rc at 126d373 with the new test and nothing else: run 43299329442 fails. The test fails on its assertion, AssertionError: 'cloud' unexpectedly found in {'javascriptProperties': [], 'sequence': {...}, 'cloud': {'cloud': '{"device":{"ismobile":true}}'}}, and the core tests give 1 failed and 479 passed.
  • Green, the head fb22e7b: run 43299354175 passes. The test passes and the core tests give 480 passed.
  • Full build of the head fb22e7b6e04c9a21b09d6f25acda8b3a9a6ed253: run 43299539908 succeeded on tmp/pr-matrix-91x, built from rc at 126d373, with all ten Build and Test jobs (Ubuntu and Windows, Python 3.10 to 3.14) green. Every one of the 11 Setting 'PullRequestSha' lines in its job logs reads the head, Configure printed "Left out 5 macOS entries", and the new test ran and passed in the Ubuntu Python 3.10 job.

Red and green ran on throwaway branches, tmp/fixpy-cloudkey-red-rc (2444207, being rc plus the test, a temporary workflow and its list of tests) and tmp/fixpy-cloudkey-green-rc (4fedea9, being the head plus the same workflow and list). The red and green runs of 10 November were on the older base fc68b1e and are replaced by these.

macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.

What remains

Nothing in this change. A page that read cloud out of the JSON no longer finds it, which the release notes should say, as item 7 of the PHP release draft (51degrees-php#76) does. #92 changes the same file at lines 175 to 247, away from the exclusion list at lines 126 to 130 that this draft changes.

The last release into main (#39) merged on 2026-11-04.

@jwrosewell
jwrosewell marked this pull request as ready for review November 12, 2026 21:10
@jwrosewell
jwrosewell merged commit 625ee4e into rc Nov 12, 2026
3 checks passed
@jwrosewell
jwrosewell deleted the fix/json-builder-leaves-out-cloud branch November 12, 2026 21:10
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