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: updated closing dangling streams at src/transit.js #1307

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 54 additions & 7 deletions src/transit.js
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@
}

if (!pass) {
isNew = true;

Check warning on line 533 in src/transit.js

View workflow job for this annotation

GitHub Actions / common

'isNew' is assigned a value but never used
this.logger.debug(
`<= New stream is received from '${payload.sender}'. Seq: ${payload.seq}`
);
Expand Down Expand Up @@ -1033,12 +1033,8 @@
// Close pending request streams of the node
this.pendingReqStreams.forEach(({ sender, stream }, id) => {
if (sender === nodeID) {
// Close the stream with error
if (!stream.destroyed) {
stream.destroy(new Error(`Request stream closed by ${nodeID}`));
}

this.pendingReqStreams.delete(id);
this._destroyStreamIfPossible(stream, `Stream closed by ${nodeID}`);
}
});

Expand All @@ -1054,12 +1050,63 @@
})
);

this.pendingReqStreams.delete(id);
this.pendingResStreams.delete(id);
this._deletePendingReqStream(id, nodeID);
this._deletePendingResStream(id, nodeID);
}
});
}

/**
* Internal method to delete a pending response stream from `pendingResStreams`
* and destroy it (if not already destroyed) with error.
*
* @param {String} id ID of the stream in `pendingResStreams`
* @param {String} origin NodeID of the origin of the destroy request
*
* @memberof Transit
*/
_deletePendingResStream(id, origin) {
const stream = this.pendingResStreams.get(id);
this.pendingResStreams.delete(id);

if (stream) {
this._destroyStreamIfPossible(stream, `Stream closed by ${origin}`);
}
}

/**
* Internal method to delete a pending request stream from `pendingReqStreams`
* and destroy it (if not already ended) with error.
*
* @param {String} id ID of the stream in `pendingReqStreams`
* @param {String} origin NodeID of the origin of the destroy request
*
* @memberof Transit
*/
_deletePendingReqStream(id, origin) {
const reqStream = this.pendingReqStreams.get(id);
const pass = reqStream ? reqStream.stream : undefined;
this.pendingReqStreams.delete(id);

if (pass) {
this._destroyStreamIfPossible(pass, `Stream closed by ${origin}`);
}
}

/**
* Internal method to destroy a stream if it is not already destroyed.
*
* @param {DuplexStream} stream - The stream to be destroyed.
* @param {String} errorMessage - The error message to be used when destroying.
*
* @memberof Transit
*/
_destroyStreamIfPossible(stream, errorMessage) {
if (!stream.destroyed && stream.destroy) {
stream.destroy(new Error(errorMessage));
}
}

/**
* Create error field in outgoing payload
*
Expand Down
Loading