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

WebSocket can be destroyed before message sent #483

Merged
merged 1 commit into from Mar 6, 2019

Conversation

Projects
None yet
3 participants
@ddgenome
Copy link
Member

commented Mar 6, 2019

There were unhandled promise exceptions in the test because tests were
creating a websocket that never was connected and so was unable to
send messages. The interval function kept running after the websocket
was destroyed, causing this to become undefined. Since this could
also adversely affect normal operation, I added a check to ensure
this was defined.

Changes in the tests are whitespace and increasing the timeout on some
recently temperamental tests.

WebSocket can be destroyed before message sent
There were unhandled promise exceptions in the test because tests were
creating a websocket that never was connected and so was unable to
send messages.  The interval function kept running after the websocket
was destroyed, causing `this` to become undefined.  Since this could
also adversely affect normal operation, I added a check to ensure
`this` was defined.
@cdupuis

cdupuis approved these changes Mar 6, 2019

Copy link
Contributor

left a comment

LGTM

@jessitron

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2019

this is clearly better. Thank you. "this" as undefined... I don't understand quite how that happens at all and would like to (shouldn't it know that it's still referenced?). That can come later

@ddgenome

This comment has been minimized.

Copy link
Member Author

commented Mar 6, 2019

As I understand it, when the setInterval callback is created this is defined so this.send is dereferenced to the location of that method. Eventually, this goes out of scope and the object gets destroyed. Perhaps it wouldn't if the timer weren't unrefed. When the callback executes it is able to call the method at the this.send memory location but when that method gets entered, there is no this so the thing barfs.

@ddgenome ddgenome merged commit e617788 into master Mar 6, 2019

1 of 2 checks passed

sdm/atomist/atomist-sdm Atomist Software Delivery Machine goals in progress
Details
license/cla Contributor License Agreement is signed.
Details

@ddgenome ddgenome deleted the websocket-lifecycle branch Mar 6, 2019

atomist-bot added a commit that referenced this pull request Mar 6, 2019

Changelog: #483 to fixed
[atomist:generated]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.