Skip to content

Conversation

@gferon
Copy link
Collaborator

@gferon gferon commented Jun 16, 2025

No description provided.

@gferon gferon requested a review from rubdos June 16, 2025 08:08
@gferon
Copy link
Collaborator Author

gferon commented Jun 16, 2025

@rubdos @direc85 wdyt? should I proceed and move all of the requests methods from PushService to their relevant SignalWebSocket<C> counterpart?

@rubdos
Copy link
Member

rubdos commented Jun 17, 2025

@gferon yes please, this looks very good to me!

@gferon gferon changed the title Experiment with more type safety around websocket Migrate REST HTTP service to using the emulated HTTP request/response in the identified websocket Jul 1, 2025
@gferon gferon changed the title Migrate REST HTTP service to using the emulated HTTP request/response in the identified websocket Migrate PushService methods to the emulated HTTP request/response in the identified websocket Jul 1, 2025
@gferon gferon changed the title Migrate PushService methods to the emulated HTTP request/response in the identified websocket Migrate PushService methods to the emulated HTTP request/response inside of the websocket Jul 1, 2025
@direc85
Copy link
Contributor

direc85 commented Oct 14, 2025

Looks like a good direction to go forward with!

@gferon
Copy link
Collaborator Author

gferon commented Oct 14, 2025

Looks like a good direction to go forward with!

Lemme try to go crazy tonight or tomorrow and move forward.

@gferon gferon marked this pull request as ready for review November 18, 2025 19:42
@gferon
Copy link
Collaborator Author

gferon commented Nov 18, 2025

@rubdos I'm pretty happy with this, you can look for reference at the other side of the story in presage (whisperfish/presage#343), but I don't think adapting things will take a lot of time in whisperfish!

@direc85
Copy link
Contributor

direc85 commented Dec 7, 2025

Note that this bumps MSRV to 1.88 (can't remember by whom exactly) but edition is still 2021.

@gferon Can you estimate how complete this PR is? This will be required soon...

@gferon
Copy link
Collaborator Author

gferon commented Dec 8, 2025

I think it's actually finished as far as I could test. See the relevant changes in presage, @Schmiddiii even tested on Flare IIRC.

@Schmiddiii
Copy link
Contributor

@Schmiddiii even tested on Flare IIRC

I indeed did. Linking, groups, and stickers worked as expected (at least as of 2025-11-19, not sure if anything changed since then), I did not try registering though as Flare does not yet support that officially. I also did not look at the code yet, but I likely don't know too much about the internals of Signal to be of much use for a review of the code.

@direc85
Copy link
Contributor

direc85 commented Dec 8, 2025

I tested linking as secondary, and in principle it worked. I'm having a little trouble creating the websockets in the right places at the right time, so I can't do much after linking. Didn't test registration as primary yet though.

@gferon
Copy link
Collaborator Author

gferon commented Dec 12, 2025

@direc85 both linking and registration didn't change 😂 but all the other operations once you register, like registering pre-keys and send/receive operations. I think we should merge this, rip the band-aid, go in the corner, sit down, cry a lot and get back on our toes afterwards.

Copy link
Contributor

@direc85 direc85 left a comment

Choose a reason for hiding this comment

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

I'm not entirely sure how and where clients (Whisperfish) should be creating websockets, but as I'm able to daily drive it, I'll approve this PR. We have to get this forward.

@gferon gferon force-pushed the one-websocket-to-rule-em-all branch from f65fb72 to c2bd0e7 Compare December 13, 2025 17:38
@gferon gferon enabled auto-merge (squash) December 13, 2025 17:38
@gferon gferon merged commit a673ad9 into main Dec 13, 2025
7 checks passed
@gferon gferon deleted the one-websocket-to-rule-em-all branch December 13, 2025 17:39
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.

5 participants