Fix Terrain Lockups in requestTileGeometry due to Promise Handling Inconsistencies

[marcejohnson]

Context

We observed that complex terrains sometimes cause CesiumJS to lock up or freeze. This issue was traced back to inconsistencies in promise handling within the requestTileGeometry function, particularly when dealing with undefined promises.

Proposed Solution

In this PR, we've enhanced the promise handling mechanism within requestTileGeometry. The updated logic ensures proper handling of undefined returns, preventing a recursive code path leading to terrain lockups.

Demonstration

We have set up a private Sandcastle example to demonstrate the problem. This can be shared upon request.

Impact

This change is crucial for applications dealing with complex terrains. It enhances the stability of CesiumJS and eliminates a cause of freezes.

Related

• Terrain with multiple layers loading higher LOD tiles inconsistantly #11312

Note: Thank you for looking at this. This PR is open for discussion and feedback from the Cesium community and maintainers.

---

Checklist

• Compatibility with all supported browsers.
• Tests pass, including coverage, release-tests, deploy, node-16, and lint.
• CLA and CHANGES.md requirements are met.

[cesium-concierge]

Thanks for the pull request @marcejohnson!

• ✔️ Signed CLA found.
• ❔ Unit tests were not updated.
  • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.

[ggetz]

Thanks for the Pull Request @marcejohnson!

We have set up a private Sandcastle example to demonstrate the problem. This can be shared upon request.

Yes, this will help with testing. Please share this with support@cesium.com and/or gabby@cesium.com. Thanks!

[marcejohnson]

Please share this with support@cesium.com and/or gabby@cesium.com. Thanks!

Done! I've sent the Sandcastle example to your email as requested, @ggetz. It demonstrates the intermittent lockup issue that the PR aims to address.

[marcejohnson]

cesium-concierge asked:

❔ Unit tests were not updated.

• Make sure you've [updated tests]

This PR addresses a break/fix without new features, emerging from specific terrains in integration tests. Existing unit tests suffice for current functionalities, so no new tests were added.

[ggetz]

Thanks @marcejohnson! Testing is working well on the dataset you supplied.

Here is an alternative data source I used for testing. This was a terrain dataset which helped us identify an issue with the previous attempted fix. The fix here does not cause any issues with loading that dataset 👍.

[marcejohnson]

@ggetz I made all the changes based upon your code review for implicit promise handling, committed these, resynced with the base branch, and re-tested against our private problem terrain. All thumbs up on our side! Please let me know if there's anything else needed.

[jjhembd]

I did a quick run-through of Gabby's testing Sandcastle, as well as the other Terrain-related Sandcastles. Everything works well on my machine.

Thanks @marcejohnson!