gfodor / p2pcf

Low cost, low effort P2P WebRTC serverless signalling using Cloudflare Workers
MIT License
964 stars 53 forks source link

fix: no remove peers #16

Closed ThaUnknown closed 1 year ago

ThaUnknown commented 2 years ago

removes peers removal from worker responses as its unreliable

gfodor commented 2 years ago

Can you elaborate on this?

gfodor commented 2 years ago

Oh, noticed it's a draft. Nvmd.

ThaUnknown commented 2 years ago

15

gfodor commented 2 years ago

Possible to update this to remove peers that aren't connected, instead of all of them? This is there basically to clean those up i/fwhen they timeout from the signalling side.

ThaUnknown commented 2 years ago

Possible to update this to remove peers that aren't connected, instead of all of them? This is there basically to clean those up i/fwhen they timeout from the signalling side.

that will remove peers who take a long time to connect, which I had happen

those probably should GC when they fail to connect?

idk how to manage either of those things tbh, I haven't looked at the methods of this lib too much

gfodor commented 2 years ago

The server timeout is 2 minutes and the ICE timeout is typically like 30s. If the peers are taking long to connect I’m guessing they never were going to connect, not that they would eventually.

gfodor commented 2 years ago

I’m up for revisiting this cleanup in the way you suggest after we merge the version that maintains active connections. I would need to set up a testing environment and so on to do a full test to make sure that doesn’t regress anything - but the simpler change where we don’t disconnect active ones seems obviously safe so let’s land that first.