Skip to content

Fixed whip test that is broken because of library change.#569

Closed
master312 wants to merge 1 commit intomasterfrom
fix/drm-cluster-play
Closed

Fixed whip test that is broken because of library change.#569
master312 wants to merge 1 commit intomasterfrom
fix/drm-cluster-play

Conversation

@master312
Copy link
Copy Markdown

@master312 master312 commented Sep 10, 2025

This fixes WHIP test (whip.html).

Submitted under 'drm-cluster-play' branch name in order to make CI pass for task ant-media/Ant-Media-Server#7439, but this fix is not directly related to DRM issue, it's a general fix.

Test page previously imported externally hosted NodeJS library which has changed. I was not able to find working version hosted on public CDS, so i added "/webapp/js/SimpleWhipClient.js" file to replace its functionality.

@mekya
Copy link
Copy Markdown
Contributor

mekya commented Sep 11, 2025

Hi @master312
Thank you for the fix. I appreciate your proactivity.

I prefer to resolve this issue by writing less code and less maintenance as in the following PR
#570

Thank you for your proactive approach

FYI
Regards

@mekya mekya closed this Sep 11, 2025
@mekya mekya deleted the fix/drm-cluster-play branch September 11, 2025 05:49
@master312
Copy link
Copy Markdown
Author

master312 commented Sep 11, 2025

Hi @mekya

I 100% agree with “less is better” philosophy, but I’ve got a couple of concerns about pulling libraries from external services in an enterprise project.

First thing I tried was exactly what you did. I just added @1.1.2 to the link. But the server kept throwing mix of 500 and 404 errors. esm.sh isn’t really “enterprise grade,” it’s run by a single person, and I don’t trust that these issues won’t pop up again in the future. The only way to be sure this doesn’t bite us later is to keep the lib directly in the project.

Second thing is security. Pulling from external sources leaves us wide open to supply chain attacks. We’ve already seen several large-scale NPM attacks this year alone, and I don’t have much confidence in relying on a small third-party service for this. Here’s the most recent one. If this were a bleeding-edge library that needs constant updates, we would have no choice, but since it’s just WHIP negotiation code that hardly changes, I don’t see the upside.

Also, by hitting esm.sh directly, we’re leaving fingerprints of our test server activity all over their logs, which isn’t great either. If ems.sh gets compromised, it might lead to a lot more headic then just copying library to project it self.

That’s why I still lean towards keeping the library in the repo. We could just copy entire '@eyevinn/whip-web-client' library into 'js/external', instead of using the hacky one i added.

Cheers :)

@mekya
Copy link
Copy Markdown
Contributor

mekya commented Sep 11, 2025

Hi @master312

Thank you for your explanation

We may discuss further in a meeting about your concerns and their impact onto us with @burak-58.

Thank you dear friend 👏 I fully appreciate the way you work 👏

Cheers
Oguz

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.

2 participants