Handle failed cloud requests gracefully in IPI cloud and translation engines - #315
Open
Jamesr51d wants to merge 1 commit into
Open
Handle failed cloud requests gracefully in IPI cloud and translation engines#315Jamesr51d wants to merge 1 commit into
Jamesr51d wants to merge 1 commit into
Conversation
…engines IpiCloudEngine previously overrode ProcessEngine and threw PipelineConfigurationException whenever the cloud response JSON was null or empty, which is the state every failed cloud request leaves behind. The message blamed pipeline ordering, misdiagnosing outages as configuration faults. It now overrides ProcessCloudEngine so the base class handles the empty response case, which logs once, marks the aspect data as cloud-request-failed for truthful missing-property reasons, and records a non-throwing flow error. CountriesTranslationEngine now also catches PropertyMissingException when reading the weighted country code properties, as that is what cloud aspect data throws when the property is not available, and it reads its own translated properties defensively so a no-value placeholder of a different type cannot fail the engine with an invalid cast. This also removes a null dereference when the initial property read had failed. Website issues 51Degrees/Website#901 and 51Degrees/Website#900 track the production impact.
Jamesr51d
marked this pull request as ready for review
July 22, 2026 11:39
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.
Contributes to 51Degrees/Website#901 and 51Degrees/Website#900 (exception noise investigated under 51Degrees/Website#885).
The problem
IpiCloudEngine overrode ProcessEngine and threw PipelineConfigurationException whenever the cloud response JSON was null or empty, which is the state every failed cloud request leaves behind. The message blamed pipeline ordering ("there is not a 'CloudRequestEngine' before the 'IpiCloudEngine'"), misdiagnosing outages as configuration faults, 15,442 occurrences in 30 days on the 51degrees.com website whose ordering is correct in all three of its IPI pipelines. Separately, CountriesTranslationEngine caught KeyNotFoundException and PipelineException around its reads of ip.CountryCodesGeographical and ip.CountryCodesPopulation but not PropertyMissingException, which extends plain Exception and is exactly what cloud aspect data throws when the property is unavailable (428 occurrences in the same window). When a catch did fire, the code then dereferenced the null result, and its typed reads of its own translated properties could fail with InvalidCastException on a string no-value placeholder.
The fix
IpiCloudEngine now overrides ProcessCloudEngine instead of ProcessEngine. These are two rungs of the base class template. ProcessEngine is the outer step that owns getting the JSON, reporting a genuinely missing CloudRequestEngine with an accurate message, and handling the empty response case by logging once, calling MarkCloudRequestFailed so missing-property reasons say the cloud request failed rather than blaming the resource key, and recording a non-throwing flow error. ProcessCloudEngine is the designated subclass hook that the base only calls with non-empty JSON. Overriding the outer rung had replaced all of that orchestration with a pre-graceful-path copy, and opted the engine out of base class improvements (device detection picked up the graceful path, IPI did not). The engine now contains only the IPI-specific work of deserializing the ip section, matching its sibling CloudCountriesTranslationEngine.
CountriesTranslationEngine adds PropertyMissingException to the caught types, treats a null read result as no value, and reads its own weighted translated properties through a new GetWeightedOrEmpty helper. The helper looks the property up via AsDictionary().TryGetValue and pattern-matches the value to the weighted list type, returning an empty list when the property is absent, holds a differently typed no-value placeholder, or has no value. Empty is the correct degradation because the All-list builder merges the weighted values with the full embedded country dictionary, so with no weights it still produces the complete alphabetical list (the ContactUs country dropdown keeps working during outages, only visitor-specific weighting is lost). Because the helper tolerates both the current string placeholder and the typed placeholder coming in 51Degrees/pipeline-dotnet#350, this change has no release-ordering dependency on that PR.
Testing
New stub-driven IpiCloudEngineProcessTests cover null, empty, and populated JSON responses without contacting the cloud, asserting no throw and that the ip element data is still added on failure. A new TranslationTests case drives the countries engine from ip data whose typed getters throw PropertyMissingException, asserting no escape and complete All lists. Cloud suite 32 passed, translation suite 15 passed, 0 failed.
Rollout
Pages that previously logged an exception during outages now render with no IPI values and truthful missing-property reasons, matching device detection. Consumers that relied on catching PipelineConfigurationException to detect outages (none known) would move to FlowData.Errors. Follow-up once 51Degrees/pipeline-dotnet#350 ships as a package, declare the weighted list placeholder types for the country translations via the new AddTranslation generic overload.