Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

feat: close connections when too many streams are opened #213

Merged

Conversation

achingbrain
Copy link
Member

If we hit the inbound stream limit and the remote peer continues to attempt to open new streams at a rate we consider to be unreasonable, close the connection.

Mplex has no notion of backpressure so this is all we can really do to protect ourselves.

If we hit the inbound stream limit and the remote peer continues
to attempt to open new streams at a rate we consider to be unreasonable,
close the connection.

Mplex has no notion of backpressure so this is all we can really
do to protect ourselves.
README.md Outdated Show resolved Hide resolved
@achingbrain achingbrain merged commit 9140770 into master Sep 7, 2022
@achingbrain achingbrain deleted the feat/close-connections-when-too-many-streams-opened branch September 7, 2022 08:54
github-actions bot pushed a commit that referenced this pull request Sep 7, 2022
## [5.2.0](v5.1.2...v5.2.0) (2022-09-07)

### Features

* close connections when too many streams are opened ([#213](#213)) ([9140770](9140770))
@github-actions
Copy link

github-actions bot commented Sep 7, 2022

🎉 This PR is included in version 5.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@fryorcraken
Copy link

rate-limiter-flexible uses a NodeJS API util, which means to use mplex in the web, we now need to handle the polyfilling of util.

It looks like it does not need to be polyfilled:

https://github.com/animir/node-rate-limiter-flexible/blob/d070b20cadca8b17246653d851256ee5f859f35b/lib/RateLimiterRes.js#L55

However, it is still makes the library less accessible as one still need to specify that util polyfill is not needed. For example. when using a React app then needs to eject or use craco to override the webpack config.

What are your thoughts on this? Can we aim to avoid using Node APIs in libp2p libs and their dependencies? Should this only be an library consumer concern?

Cheers.

fryorcraken added a commit to waku-org/js-waku that referenced this pull request Sep 12, 2022
fryorcraken added a commit to waku-org/js-waku that referenced this pull request Sep 12, 2022
@fryorcraken
Copy link

It looks like it does not need to be polyfilled:

I was incorrect, it does need to be polyfilled as util.inspect.custom is called.

Uncaught TypeError: util.inspect is undefined
    35398 RateLimiterRes.js:55

fryorcraken added a commit to waku-org/js-waku that referenced this pull request Sep 13, 2022
@fryorcraken
Copy link

Actually got resolved overnight: animir/node-rate-limiter-flexible#179

@wemeetagain
Copy link
Member

Yeah, ideally we don't rely on nodejs apis in our libraries. This one slipped by me

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants