Fix: TaskProcessor cross-origin worker loading

[harrythemorris]

Description

Fixes two issues that we have come across when upgrading to the latest version of Cesium with our unusual setup, where our application bundle and Cesium resources are loaded cross-origin.

• The createWorker function uses the module ID rather than a full URI when considering whether to load the worker with a cross-origin shim. This results in e.g. SecurityError: Failed to construct 'Worker': Script at 'http://localhost:8080/cesiumStatic/Workers/createVerticesFromQuantizedTerrainMesh.js' cannot be accessed from origin 'http://localhost:3003'.

• The cross-origin shim used the importScripts method does not support ESM modules. It will fail at runtime with error Uncaught TypeError: Failed to execute 'importScripts' on 'WorkerGlobalScope': Module scripts don't support importScripts().

Issue number and link

Could not find any related issues.

Testing plan

A minimal reproduction is available here.

To reproduce issue:

• Setup a Cesium project where the bundled application, as well as all Cesium assets (including workers) are hosted on domain A, and the application is initialised cross-origin on domain B.
• Cesium will attempt to load the correct URL, but without the cross-origin shim.
• If the above is fixed, the workers are then loaded with an importScripts shim, which gives vague and unhelpful errors when loading ESM modules.

The net result is Cesium unable to render the globe, erroring out before any display.

With the fixes, it should initialise as usual.

Author checklist

• I have submitted a Contributor License Agreement (Via PropellerAero corporation CLA)
• I have added my name to CONTRIBUTORS.md
• I have updated CHANGES.md with a short summary of my change
• I have added or updated unit tests to ensure consistent code coverage
• I have update the inline documentation, and included code examples where relevant
• I have performed a self-review of my code

[ggetz]

Hi @harrythemorris, I don't see a CLA on file on file for you. We'll need this before we can review this PR. See CONTRIBUTING.md for instructions on submitting a CLA.

Let us know once that's submitted. Thanks!

[harrythemorris]

Hi @harrythemorris, I don't see a CLA on file on file for you. We'll need this before we can review this PR. See CONTRIBUTING.md for instructions on submitting a CLA.

Let us know once that's submitted. Thanks!

Hi @ggetz sorry for the trouble! My employer PropellerAero have signed a corporation CLA - are there any steps that I need to take to verify that I am a member of the org?

[ggetz]

No worries @harrythemorris! I see PropellerAero, but I don't see your user ID or name listed under "Schedule A". Would you be able to have your admin or point of contact update the list?

[jaycosaur]

Heyo @ggetz! I'm the CTO of PropellerAero and we haven't contributed here in a little while as an org so that list is quite out of date. How would we go about updating it for Propeller Aero? The best contact on that list to confirm details would be @keattang if you could send email correspondence to him?

[ggetz]

@jaycosaur @keattang The simplest approach would be to re-submit the Corporate CLA with new usernames.

[ggetz]

Actually, a vouch from @keattang via a comment here would be enough.

[keattang]

Hi @ggetz! I vouch for both @jaycosaur and @harrythemorris. If they can be allowed to vouch for other people that would be great too. Let me know if you'd like me fill out any forms or need any other details 😄

[ggetz]

Thanks for the PR @harrythemorris!

I seem to be having trouble replicating the issue. Would you mind giving us a few more details about how the application is being bundled?

[ggetz]

Would it possible to call dummy code instead such that this doesn't throw? For example, could the blob spy return a blobbed dummy script?

[harrythemorris]

I've added the ability to skip running the task for testing purposes in 20d9671

Mocking out a dummy worker script with necessary message handlers so that it doesn't hang got a bit bulky and I think this results in a cleaner test, but happy to revisit if you'd prefer to avoid this approach

[ggetz]

Could you please add a link to this PR given there is no original issue.

[harrythemorris]

Thanks for the PR @harrythemorris!

I seem to be having trouble replicating the issue. Would you mind giving us a few more details about how the application is being bundled?

Hi @ggetz I've put together a minimal repro here based on the cesium-webpack-example.
https://github.com/harrythemorris/cesium-webpack-example/tree/cors-worker-repro

Essentially:

• Origin A holds our application bundle as well as cesium static assets. It has appropriate CORS headers set to allow the resources to load, and this worked very happily with Cesium 1.99 (presumably things broke with ESM but I cannot confirm)
• Origin B loads the application bundle, and calls a well-known function it exports to start the application.

This webpack setup emulates this, with the webpack dev server standing in for Origin A, serving the bundle and static assets, and the test-5 script starting up a server to act as origin B loading our bundle.

I've updated the PR description with a link to this, and have added screenshots of the errors that occur. Let me know if you have any more trouble reproducing, and I can try to get this setup somewhere publicly accessible.

[ggetz]

Thanks for the test repo @harrythemorris! I can reproduce the runtime error!

When I use this branch in that example instead of main, the runtime rendering error goes away, but I'm still seeing CORS errors in the console for other assets. I understand this PR focuses on workers, but is this expected?

[harrythemorris]

Thanks for the test repo @harrythemorris! I can reproduce the runtime error!

When I use this branch in that example instead of main, the runtime rendering error goes away, but I'm still seeing CORS errors in the console for other assets. I understand this PR focuses on workers, but is this expected?

Hi @ggetz

Not expected! I am not seeing those errors here when running on my machine - all static assets are loading successfully (e.g. skybox, workers, widgets/). To test my branch, I used npm link after doing a release build.

[ggetz]

@harrythemorris I tried again, and I'm still seeing the CORS errors in Chrome.

I'm not seeing them in Firefox though.

In this branch, I ran

npm install
npm run make-zip
npm link

and in your provided sample repo I ran

npm link cesium
npm run build
npm run start:built
npm run start:client

Strictly speaking this doesn't need to hold up this PR, but I want to make sure we're addressing the core issue before merging.

[harrythemorris]

@harrythemorris I tried again, and I'm still seeing the CORS errors in Chrome.

I'm not seeing them in Firefox though.

In this branch, I ran

npm install
npm run make-zip
npm link

and in your provided sample repo I ran

npm link cesium
npm run build
npm run start:built
npm run start:client

Strictly speaking this doesn't need to hold up this PR, but I want to make sure we're addressing the core issue before merging.

@ggetz thanks again for taking the time to try this out 🙏

I believe the issue stems from npm run start:built -> if you run just npm run start from the webpack-5 dir instead, it will serve via webpack, which has CORS headers enabled. I had not changed the start:built script to do so, but just pushed a change so that would work now.

(I've also resolved the conflict and moved my CHANGES to next release)

[harrythemorris]

Hi @ggetz just wanted to bump this one - I believe the above concerns have been addressed, and I've resolved all conflicts ready to merge.

[ggetz]

Thanks for your patience @harrythemorris! I tweaked the unit test to remove the reliance on a _skipTaskRun flag. Once CI passes, this should be good to merge.