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

fix: stream close #116

Merged
merged 3 commits into from
Oct 22, 2020
Merged

fix: stream close #116

merged 3 commits into from
Oct 22, 2020

Conversation

jacobheun
Copy link
Contributor

@jacobheun jacobheun commented Oct 21, 2020

This adds some more aggressive closures to streams on abort and reset. It was previously possible for unwritten streams to not properly end when aborted/reset, which would result in them not properly notifying libp2p/application code of the termination. This could also result in streams lingering.

  • This also includes some formatting updates needed due to the Aegir bump

I'd like to do some more clean up here but am reserving that for #115, which will create some breaking behavior.

This should also resolve part of #113.

Copy link
Member

@achingbrain achingbrain left a comment

Choose a reason for hiding this comment

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

LGTM, just some minor nits.

src/stream.js Outdated Show resolved Hide resolved
test/stream.spec.js Outdated Show resolved Hide resolved
test/stream.spec.js Outdated Show resolved Hide resolved
test: verify abort/reset errors are correct
@jacobheun jacobheun requested a review from achingbrain October 21, 2020 15:33
@jacobheun
Copy link
Contributor Author

Added the assertions and removed the unused error param from the abort call, should be good now.

@jacobheun jacobheun merged commit 77835b3 into master Oct 22, 2020
@jacobheun jacobheun deleted the fix/stream-close branch October 22, 2020 11:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants