Repository navigation
Discover when the Cloud Request Engine is built, and retry on first use only after a failure that can pass - #86
Merged
Conversation
…tries only after a failure that can pass CloudRequestEngineDiscoveryTests uses a stand-in for the HTTP client that counts the requests to each endpoint. A build fetches the accessible properties and the evidence keys once each and processing fetches neither again. A 4xx answer to either fails the build with a CloudRequestException carrying the status, once both requests are made, also when the other request failed in a way that can pass. A 5xx answer, a body that cannot be read, a refused connection and a timeout each complete the build with one warning. A list that arrived is kept, processing fetches only what is missing and sends no data request until both are known, the Pipeline's Evidence filter sees the keys a retry fetched, and the configured timeout applies to the discovery requests. The existing tests that expected getPublicProperties and getEvidenceKeyFilter to fetch and throw when the server was unavailable, and a 200 answer holding errors to throw, now expect the build to complete with one warning. The cloud aspect engines are tested to build without logging when the accessible products are not known yet, and to read their metadata once they are.
…se only after a failure that can pass The engine fetched its accessible properties and accepted evidence keys only when something first asked for them, and a failure surfaced wherever that was. The builder now calls discover, which makes both requests. A 4xx answer to either is a definitive configuration error, and its CloudRequestException fails the build once both requests are made. Any other failure, the service unreachable, a timeout, a 5xx answer or a body that cannot be read, is logged as one warning, the build completes and the list that arrived is kept. The configured timeout now applies to both requests. getPublicProperties and getEvidenceKeyFilter make no request. The products are null until they are known. The evidence filter is one CloudEvidenceKeyFilter that includes nothing until the keys are known and reads them on each call, and it derives from EvidenceKeyFilterWhitelist so the Pipeline's filter keeps it rather than copying its keys once. Processing fetches what is missing, evidence keys first, before it sends the data request, and a request that could not be made is reported as its IOException, as for the data request. The Device Detection, IP Intelligence and Location cloud engines, and CloudAspectEngineBase, read their metadata when the products are known rather than logging errors at build and keeping none. A missing product still builds and logs once. The web integration reads a filter that derives from EvidenceKeyFilterWhitelist for the Vary header, as it reads the exact class.
…er one discovered The specification's Resilience section says an engine SHOULD be buildable from a kept copy of the discovery results, the accepted evidence keys and the accessible properties, which depend only on the resource key, so that a Pipeline can be built offline and a host that starts often need not call the cloud service each time. CloudDiscoveryState holds the evidence keys and the accessibleproperties answer, and writes and reads them as one JSON document. CloudRequestEngineBuilder.getDiscoveryState returns the copy after a build that knows both lists, and setDiscoveryState makes a later build use it with no discovery request. The tests are in this commit because they use the new class and methods.
…conds, and that the Pipeline reads metadata again once it is known Each discovery request in the new test waits for the other to start, so they only succeed if they are made at once. A build with no timeout set applies 2 seconds to both, as the page's Configuration options table and .NET give. The Pipeline's map of available properties, built before the Cloud Request Engine knows the accessible properties, holds the cloud aspect engine's properties once they are known. The shared test client now gives each request a connection of its own, as the requests can be made at the same time.
…ult, and read metadata again until it is loaded The two discovery requests are now made at once, as the specification allows, so a service that does not answer delays the build by one timeout rather than two. The evidence keys request runs on another thread while the accessible properties request runs on the building thread, and the build waits for both. The default timeout for each request to the cloud service is now 2 seconds, as the specification's Configuration options table gives and as .NET's CLOUD_REQUEST_TIMEOUT_DEFAULT_SECONDS is. It was 100 seconds. FlowElement gains hasLoadedProperties, true by default, as .NET's aspect engines have HasLoadedProperties. The Cloud Request Engine returns true once both discovery lists are known, the cloud aspect engines once their metadata is loaded, and a parallel group once all its elements have. PipelineDefault keeps its map of available properties only once every element has loaded its properties, and builds it again on each call until then, as .NET's Pipeline.ElementAvailableProperties does after a PropertiesNotYetLoadedException. The web integration works out the Vary headers again on each response until every element has loaded, as .NET's ClientsidePropertyService.HeadersAffectingJavaScript does after an exception from an element's EvidenceKeyFilter.
…lement has loaded its properties With every element loaded, the configuration built from the Pipeline's available properties is built once and kept across requests. While an element has not loaded its properties, it is built again on each request, and kept once every element has loaded. Both fail on the code before the fix, where the configuration was built again on every request, because getOrDefault never stored it.
…aded its properties The SetHeaders element built its configuration from the Pipeline's available properties on every request, because getOrDefault never stored it. It now keeps it once every element of the Pipeline has loaded its properties, and builds it again on each request until then, as after a cloud element could not reach its service when the Pipeline was built. So the configuration gains the cloud's SetHeader properties once a retry fetches them, and is then built once. This is the same rule the Pipeline's map of available properties and the web integration's Vary header follow.
…eats raw types as errors
thenReturn(null, products) uses the varargs overload, and a generic Map array is an unchecked warning that the build treats as an error, so each stub now chains two single-value thenReturn calls.
…e build The specification now treats a 408 or a 429 answer to a discovery request as a transient failure, because the same request can succeed later, so 429 moves from the cases that fail the build to those that do not, and 408 cases join it.
… build A build fails on a 4xx answer because no retry with the same configuration can succeed, which is not so for a 408 or a 429, as RFC 9110 section 15.5.9 and RFC 6585 section 4 say, so those two are logged as a warning and retried on first use like a 5xx.
…fail the build" This reverts commit 7bc481c.
… fail the build" This reverts commit 40452b7.
The specification on rc makes every 4xx answer to a discovery request a definitive configuration error, so the cases that fail the build now include 408 beside 429.
A 4xx answer is a definitive configuration error whatever its body holds, such as an error page from a proxy. The accessibleproperties answer is read as JSON before its status is used, so a 403 with an HTML body there fails this test, as the build completes with a warning.
An answer that cannot be read as JSON now raises a CloudRequestException carrying its HTTP status, as .NET does, so a 4xx answer to a discovery request fails the build whatever its body holds, as the specification says.
This was referenced Nov 10, 2026
Merged
The one conflict is in CloudRequestEngineDefault.java, where rc adds getLicenseKeys() in the place where this branch adds setTimeouts(HttpURLConnection) and renames getCloudProperties to fetchPublicProperties. Both helpers are kept, and the accessible properties request keeps this branch's name, fetchPublicProperties, which every call site uses.
This branch carried the missing product change as it was at 33a13fe. Its head, 9fced9b, logs a missing product as a warning, treats a missing response part as no values, and removes isProductMissing. The conflicts are in the Device Detection and IP Intelligence ProductMissingTests and in GeoLocationCloudEngineTests. Each takes the file as it is at 9fced9b and keeps this branch's DiscoveryNotComplete_LoadsTheMetadataLater test above buildEngine.
jwrosewell
marked this pull request as ready for review
November 10, 2026 13:49
jwrosewell
marked this pull request as draft
November 10, 2026 16:01
jwrosewell
marked this pull request as ready for review
November 10, 2026 16:26
Draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Java Cloud Request Engine now follows the Revised design (eager discovery at build time, transient-failure fallback) section of
pipeline-specification/pipeline-elements/cloud-request-engine.mdon the specifications'rc, and offers the kept copy its Resilience section asks for.This branch carries #84 (a cloud engine whose product the resource key lacks), because the cloud aspect engines here build on the
findProductthat #84 adds. #84's head, 9fced9b, was merged in, and #84 has since merged intorcas 0db9c2e. #85 (license key masking) is merged intorcas fc55341, andrcat fc55341 is merged in here with its one conflict resolved, as the "Merges" section below says. The commits of this change are c72baac, a4b6fd5, 4defa2a, 4f62134, bef79b0, be26bcf, 9f35ac9, ea5fc04, 8f709b4, 40452b7, 7bc481c, 726204b, 19cc0fa, 1e1dae8, 127b173 and 7d4cebd, with the merge commits c309b58, 66326a3, 5a6fc84 and 8a19907. The head is 8a19907.The problem
On
rcat 7299688,CloudRequestEngineDefaultfetched its accessible properties and its accepted evidence keys only when something first asked for them, and threw where that was.getEvidenceKeyFilterthrew from whatever read it, such as the web integration's start.What the specification says
From Revised design:
From Consequences for consumers of the Engine:
From Resilience:
The Java builder refuses a build with no resource key, so Discovery under a License Key does not arise. The page notes that Recovery Mode is supported by .NET only, and the Java engine has no recovery mode, so there is no failure count for discovery to add to.
What changed
CloudRequestEngineBuildercallsdiscover, which makes both requests at once, as the page allows, so a service that does not answer delays the build by one timeout. The evidence keys request runs on another thread while the accessible properties request runs on the building thread (bef79b0).CloudRequestException, whosegetHttpStatusCode()is the cloud's status. Both requests are made first, so a 4xx on one fails the build even when the other failed in a way that can pass. The exception is the onegetPublicPropertiesthrew before, as in .NET.CloudRequestExceptionraised for it keeps the status (7d4cebd).processEnginefetches only what is missing, evidence keys first, stopping at the first failure, and sends no data request until both are known. The failure is thrown or recorded asSuppressProcessExceptionssays. A request that could not be made is reported as itsIOException, as a failed data request is.process-startedis now set before discovery, so the cloud aspect engines report a missing response rather than a missing Cloud Request Engine.TimeoutSecondsand as .NET'sCLOUD_REQUEST_TIMEOUT_DEFAULT_SECONDSis. .NET applies it to the oneHttpClientthat makes the discovery and the data requests (CloudRequestEngine.cs lines 265 to 272), and Java applies it to each connection.getPublicPropertiesandgetEvidenceKeyFilterfetched and threwgetPublicPropertiesis null until the products are known.getEvidenceKeyFilteris oneCloudEvidenceKeyFilter, which includes nothing until the keys are known and reads them on each call. It derives fromEvidenceKeyFilterWhitelist, so the Pipeline'sEvidenceKeyFilterAggregatorkeeps it rather than copying its keys once, and the Pipeline's filter sees the keys as soon as a retry fetches them.CloudAspectEngineBase.getProperties, return empty metadata while the products are unknown, without keeping it or logging, and read it once they are known. A missing product, #84's case, still builds with empty metadata and logs once, becausefindProducttells an unknown product list (null) from one without the product.CloudDiscoveryStateholds the evidence keys and theaccessiblepropertiesanswer, withtoJsonandfromJson.CloudRequestEngineBuilder.getDiscoveryState()returns it after a build that knows both lists, andsetDiscoveryStatemakes a later build use it with no discovery request.The web integration's
ClientsidePropertyServiceCorenow reads any filter that is or derives fromEvidenceKeyFilterWhitelistfor theVaryheader. It read only the exact class, so without this the engine's new filter would have dropped the cloud's headers fromVary. #67 (caching the client-side endpoints' answers) makes the same change to those lines. The two still conflict on theVaryline of the 200 answer, which this branch changes to readgetHeadersAffectingJavaScript()and #67 moves into its newsetCachingHeaders. Whichever of the two merges second keeps #67'ssetCachingHeadersand has it read the list throughgetHeadersAffectingJavaScript()in place of theheadersAffectingJavaScriptfield, for both the empty check and the value, so that the 200 and the 304 both carry headers that are worked out again until every element has loaded its properties.Compared with .NET and Go
rc) callsWarmUpfromCloudRequestEngineBuilder.Build, makes both requests in parallel, rethrows a 4xx and logs anything else as a warning. Java rethrows the same exception type and logs the same way.rcas 38acaf4) does the same, withBuilder.DiscoveryStateandSetDiscoveryStatefor the kept copy, whichgetDiscoveryStateandsetDiscoveryStatematch.getPublicPropertiesmakes no request in Java, where .NET's getter fetches on first access. In Java the cloud aspect engines and the missing property service read the products many times on each request, and a getter that fetched would make a request each time during an outage. Processing fetches them instead.Consumers that keep what they read when the Pipeline is built
After a failure at build that can pass, the page says consumers "MUST NOT permanently cache an empty result obtained before discovery has completed". Java now meets this for every consumer that keeps what it reads from the Pipeline, being the Pipeline's map of available properties, the web integration's
Varyheaders and the SetHeaders element, which closes the consumer rule fully for Java. .NET meets it for the first two, and its SetHeaders element is being changed to match.FlowElementgainshasLoadedProperties(), true by default (FlowElement.java line 115), as .NET's aspect engines haveHasLoadedProperties(CloudAspectEngineBase.cs line 247). The Cloud Request Engine returns true once both lists are known, the cloud aspect engines once their metadata is loaded, and a parallel group once all its elements have.Pipeline.ElementAvailablePropertiesis kept only when no element'sPropertiesthrewPropertiesNotYetLoadedException(lines 647 to 657), and is built again on the next call otherwise. Java'sPipelineDefault.getElementAvailablePropertiesnow keeps the map only once every element has loaded its properties, and builds it again on each call until then.getMetaDataForPropertyalready looked properties up on demand and kept only those it found, as .NET does.ClientsidePropertyService.HeadersAffectingJavaScriptkeeps the headers only when no element'sEvidenceKeyFilterthrew. Java'sClientsidePropertyServiceCorenow works them out again on each response until every element has loaded its properties.SetHeadersElement.processInternalnow keeps the configuration it builds from the Pipeline's available properties once every element has loaded its properties, and builds it again on each request until then. Before, it built the configuration on every request, becausegetOrDefaultnever stored it, so it was never stale but was rebuilt each time. .NET'sSetHeadersElement.ProcessInternalkeeps the configuration from the first request for each Pipeline without that check, and is being changed to match, so the two stay aligned.Not changed
errorsandwarningsare as before.For the release notes
CloudRequestException, rather than the first use.setTimeOutSecondsto keep a longer one.Varyheader and the SetHeaders element's response headers gain the cloud's properties and headers once a retry fetches them.FlowElement.hasLoadedProperties()says whether an element knows all its property metadata and accepted evidence. It is true by default.getPublicPropertiesandgetEvidenceKeyFilterno longer make requests.getPublicPropertiesreturns null until the properties are known.CloudRequestExceptionthat carries the HTTP status.CloudRequestEngineBuilder.getDiscoveryStateandsetDiscoveryState, withCloudDiscoveryState.toJsonandfromJson, let a host keep what one build discovered and build later with no discovery request.How to verify
No test calls a live service, apart from the existing
CloudRequestEngineTests.HttpDataSetInException.rc(fc55341)CloudRequestEngineDiscoveryTestsCloudRequestEngineTestsCloudAspectEngineBaseTestsClientsidePropertyServiceTests(pipeline.web)SetHeadersElementTests(pipeline.engines.fiftyone)ProductMissingTests(device-detection.cloud)ProductMissingTests(ip-intelligence.cloud)GeoLocationCloudEngineTests(geo-location.cloud)CloudRequestEngineTestshas one more method onrcthan at #84's head, because #85 addedProcess_LicenseKeyInCloudError_Masked.CloudRequestEngineDiscoveryTests.Build_4xx_FailsTheBuildruns 12 cases, being 400, 401, 403, 404, 408 and 429 from each request,Build_4xxWithBodyNotJson_FailsTheBuildruns two, andBuild_TransientFailure_CompletesWithOneWarningruns seven. The class uses a stand-in HTTP client that counts the requests to each endpoint.c72baac holds the tests alone, on Build a cloud engine whose product the resource key lacks, and log that once #84's code. These are written to fail there:
CloudRequestEngineDiscoveryTestsat that commit.CloudRequestEngineTests, the three cases ofvalidateErrorHandling,getPublicProperties_Server_Unavailable_At_BuildandgetEvidenceKeyFilter_Server_Unavailable_At_Build. These used to expect a throw on first read and now expect a build that completes with one warning.CloudAspectEngineBaseTests.loadProperties_notLoaded, which used to expect a throw.DiscoveryNotComplete_LoadsTheMetadataLaterin each of the three cloud aspect engine classes.a4b6fd5 holds the fix.
4defa2a adds the kept copy with its two tests,
DiscoveryState_KeptAndUsedWithNoRequestandDiscoveryState_NoneWithoutBothLists. They are in the same commit because they use the new class and methods.4f62134 holds three more tests alone, on 4defa2a's code, each written to fail there:
Build_MakesBothRequestsAtOnce. Each discovery request waits for the other to start, so made one after the other the first waits 5 seconds and fails.Build_DefaultTimeoutIsTwoSeconds, which finds 100 seconds there.CloudAspectEngineBaseTests.elementAvailableProperties_ReadAgainOnceKnown, where the Pipeline kept the empty map it built.It also gives each request in the shared test client a connection of its own, as the requests can now run at the same time.
bef79b0, holds the three fixes and
ClientsidePropertyServiceTests.JavaScript_VaryHeader_ReadAgainUntilLoaded. That test is in the same commit because it useshasLoadedProperties.be26bcf holds two SetHeaders tests alone, each written to fail on bef79b0's code, where the configuration was built on every request:
setHeadersElement_ConfigKeptOnceLoadedandsetHeadersElement_ConfigBuiltAgainUntilLoaded. They count the reads of the Pipeline's available properties.9f35ac9 holds the SetHeaders fix.
Later commits
thenReturncalls in the three cloud engine tests, becausethenReturn(null, products)made an unchecked generic array, which the build treats as an error.rcand .NET (CloudRequestEngine.cs lines 496 to 501 at 5b4d06e) both treat every 4xx at discovery as fatal, and specifications#126 was closed without being merged, so 726204b and 19cc0fa revert them.Build_4xx_FailsTheBuildexpects to fail the build, as the specification onrcmakes every 4xx at discovery definitive.Build_4xxWithBodyNotJson_FailsTheBuildalone, on 1e1dae8's code. It answers one discovery request with a 403 and an HTML body, and expects the build to fail with that status.validateResponsenow raises aCloudRequestExceptionthat keeps the HTTP status when the body cannot be read as JSON, as .NET does, so a 4xx at discovery fails the build whatever its body holds. The message names only the status, and the cause is theJSONException.Merges
rcat fc55341, which now holds Show a license key only by its last 4 characters in logs and error messages #85 (license key masking). The one conflict is inCloudRequestEngineDefault.java, where Show a license key only by its last 4 characters in logs and error messages #85 addsgetLicenseKeys()in the place where this branch addssetTimeouts(HttpURLConnection)and renamesgetCloudPropertiestofetchPublicProperties. Both helpers are kept, and the accessible properties request keeps this branch's name,fetchPublicProperties, which every call site uses.validateResponsemasks the license key in the cloud's messages as Show a license key only by its last 4 characters in logs and error messages #85 does, and the message this branch adds for a body that is not JSON names only the status.isProductMissing. The conflicts are in the Device Detection and IP IntelligenceProductMissingTestsand inGeoLocationCloudEngineTests. Each takes the file as it is at 9fced9b and keeps this branch'sDiscoveryNotComplete_LoadsTheMetadataLatertest. After the merge nothing callsisProductMissingorgetCloudProperties.rcat 24c6f25, after Build a cloud engine whose product the resource key lacks, and log that once #84 merged as a merge commit and Write weighted values with the lower-case keys rawweighting and value #93 as a squash. GitHub reported a conflict from the two merge bases this left, although git merges the two cleanly. The merge brings in only Write weighted values with the lower-case keys rawweighting and value #93'sJsonBuilderElement.javaandJsonBuilderTests.java, which this branch never touched, and every file of this branch is unchanged.Neither merge deletes a file.
Red runs
Run 42729019549 runs each set of tests on the code before its fix, in five jobs, and each job fails as expected.
CloudRequestEngineDiscoveryTestsmethods, the threevalidateErrorHandlingcases, both_Server_Unavailable_At_Buildtests,loadProperties_notLoadedand the threeDiscoveryNotComplete_LoadsTheMetadataLatertests.accessiblepropertiesonly, because theevidencekeysrequest did not read its body as JSON and already raised aCloudRequestExceptionfor a 4xx.Full build
The full build and test matrix passed every job other than macOS on head 8a19907 in run 42828763704 on 10 November 2026, run as a dry run so nothing merged. Configure and all nine build jobs passed, and all 10
Setting 'PullRequestSha'lines are 8a19907. The earlier build on 5a6fc84 was run 42800334955. 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.