Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix WS/SSE tests and replace go backend with NodeJS #1876

Merged
merged 14 commits into from
Oct 12, 2023
Merged

Conversation

alexpetros
Copy link
Collaborator

@alexpetros alexpetros commented Oct 9, 2023

Background

The live WebSocket/Server-Side Events tests were causing us a couple problems:

  • Every release that we cut we'd get flooded with dependabot spam for the golang dependencies
  • The golang dependencies also spam our GitHub security tab, where all but 4 of them are from go
  • A couple of the tests were allegedly not working (I didn't actually verify this myself)
  • You needed to install the golang toolchain to run them, which isn't a requirement anywhere else in the project

So, to the fix the dependabot thing and partially as an excuse to learn what WebSockets are, I reimplemented the tests' backend in NodeJS.

Description

The main changes are as follows:

  • I removed all the golang dependencies, at the cost of adding a single JS one (the excellent ws library). This resolves the dependabot/security issue, and removes a full language toolchain from htmx.
  • I re-implemented the routing mechanism to use hard links instead of hx-boost and hx-get, so now refreshing the page takes you back the tab you were on, instead of the homepage, and links are persisted in the URL bar/history. This makes developing on them much nicer.
  • The tests use relative URLs so no CORS headers are required, and the syntax is a little easier to follow
  • You can run the test suite from package.json now
  • The code is actually a bit smaller line-wise

And as far as I can tell, all the tests work now! There's a couple small changes to the timing of the events that I'll play around with, but what I have now gets the job done.

Testing

Run npm run ws-tests from the package root! There's also a readme in /tests/ws-sse directory with some granular instructions.

Note that the old go code is still mirrored on the website, which I left in for this PR so that the comparisons are more obvious. When we cut the next release it will go away and officially remove the the last of the go code.

Credits

Huge props to @benpate for writing the initial version of these tests. It's much easier to adapt exist code than write a whole new paradigm, and his test suite and server code was really easy to follow.

@alexpetros alexpetros added go Pull requests that update Go code devops Changes to the repository structure or project management labels Oct 9, 2023
Copy link
Collaborator

@Renerick Renerick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@benpate
Copy link
Collaborator

benpate commented Oct 10, 2023

Hey, this looks really cool. And Javascript will probably be a better / more portable environment to do this than Go has been. Awesome work :)

@alexpetros
Copy link
Collaborator Author

That means a lot, thank you Ben :)

@alexpetros alexpetros added the ready for review Issues that are ready to be considered for merging label Oct 10, 2023
@nickchomey
Copy link
Contributor

nickchomey commented Oct 10, 2023

Amazing work.

Perhaps it is irrelevant for these purposes, but https://github.com/uNetworking/uWebSockets.js/ is known to be by far the fastest/most powerful nodejs websockets library. It might be worth taking a look, as it would be a useful tool for production as well.

The author's blog is quite informative (if inflammatory at times). https://unetworkingab.medium.com/the-story-of-uws-so-far-493ac0c05ccb

Here's another good one as well https://edisonchee.com/writing/intro-to-%C2%B5websockets.js/

@alexpetros
Copy link
Collaborator Author

alexpetros commented Oct 10, 2023

@nickchomey That's great to know, these are good reads. I do think for the test purposes that I'll stick with ws—it's popular and has no dependencies, so I feel pretty good about that it will correctly run the tests more or less indefinitely, which is my main priority here.

@1cg 1cg merged commit a66a98b into master Oct 12, 2023
1 check passed
@alexpetros alexpetros deleted the replace-go-webserver branch October 12, 2023 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devops Changes to the repository structure or project management go Pull requests that update Go code ready for review Issues that are ready to be considered for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants