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

Add ability to revoke a socket ignoring if it is already closed #284

Merged
merged 2 commits into from May 30, 2019

Conversation

@rankida
Copy link
Contributor

rankida commented May 27, 2019

In cases where there is an onUnsubscribe handler registered it is possible that revoke can be called on a socket that has since been closed. This results in an error being thrown.

To prevent having to try/catch and explicitly test for nes's internal Socket not open error this new flag throwIfClosed allows the user to signal their intent to "fire and forget".

Because revoke is async internally it is not possible to catch this with certainty prior to making the revoke call.

The added tests demonstrate the issue we are seeing in production.

@rankida rankida force-pushed the rankida:revoke-closed-ws branch from 7669a6d to 6e63077 May 27, 2019
@@ -931,7 +931,8 @@ describe('Client', () => {
const a = { b: 1 };
a.a = a;

const err = await expect(client.request({ method: 'POST', path: '/', payload: a })).to.reject('Converting circular structure to JSON');
const err = await expect(client.request({ method: 'POST', path: '/', payload: a })).to.reject();
expect(err.message).to.startWith('Converting circular structure to JSON');

This comment has been minimized.

Copy link
@rankida

rankida May 27, 2019

Author Contributor

This is a fix for node 12, as seen https://travis-ci.org/hapijs/nes/jobs/537797710

This is unrelated to my PR.

image

@rankida

This comment has been minimized.

Copy link
Contributor Author

rankida commented May 27, 2019

I am going to blame Travis for the failing builds.

image

This PR now has the same success (or a little more) than master appears to have.

@hueniverse hueniverse self-assigned this May 30, 2019
@hueniverse hueniverse added this to the 11.1.1 milestone May 30, 2019
@hueniverse hueniverse merged commit ba4c27b into hapijs:master May 30, 2019
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
hueniverse added a commit that referenced this pull request May 30, 2019
@rankida rankida deleted the rankida:revoke-closed-ws branch May 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.