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

Added beat listener to call agent.ping() for state #11

Merged
merged 10 commits into from
Jun 13, 2012
Merged

Conversation

davglass
Copy link
Member

agent.ping() updates the state of the agent
to show that the agent is alive internally.
This is the first step to getting a proper
agentDisconnect event.

    agent.ping() updates the state of the agent
    to show that the agent is alive internally.
    This is the first step to getting a proper
    agentDisconnect event.
@davglass
Copy link
Member Author

Adding tests shortly, just getting this started so I can add to it.

@travisbot
Copy link

This pull request passes (merged 63b7977 into 661028c).

Agent.prototype.ping = function () {
this.connected = true;
this.seen = new Date();
this.emit('beat');
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: Double quotes in this project. Otherwise, this is looking good so far!

@travisbot
Copy link

This pull request passes (merged 989113e into 661028c).

@travisbot
Copy link

This pull request passes (merged 544f6ea into 661028c).

@travisbot
Copy link

This pull request passes (merged bf6be48 into 661028c).

The `onbeforeunload` event will trigger a
dynamic image that loads a url like:
`/ping/unload/AGENT_ID`

This will allow the server to set the agent
to a disconnected state so it won't be processed
any longer.

This is safer than a socket connection or an
XHR request since it's a simple ping with a
dead image.
@davglass
Copy link
Member Author

@reid This latest addition handles a browser leaving the page with the beforeunload event. I need to simulate a crash to see if that catches it, but it should catch windows closing or navigating away (when not by navigated by yeti).

@travisbot
Copy link

This pull request passes (merged 9c731ec into 661028c).

@@ -280,6 +280,14 @@ Hub.prototype._onRunConnection = function (socket) {
socket.emit("ready", id, function () {
self.debug("Recv");
});

socket.on("beat", function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

The correct place to put this listener is inside Agent#setupEvents in lib/hub/agent.js. When you listen against that method's self.socketEmitter, you're listening on an EventYoshi that has only the current socket EventEmitter attached. (The socket is attached to the socketEmitter by AgentManager#connectAgent.) This prevents leaks of sockets as agents connect and disconnect frequently, and it prevents breakage as I merge this into the sockjs branch.

I'm taking care of it, so just FYI.

reid added a commit to reid/yeti that referenced this pull request Jun 13, 2012
@reid reid merged commit 9c731ec into yui:master Jun 13, 2012

//Keeps it safe from outside execution
if (agent) {
switch (action) {
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: make lint complains about this switch, so I fixed this today.

The frontend stuff in lib/hub/view/public currently fails the linter pretty hard, but I try to keep the Node.js side of things clean. I hope to resolve the linter errors in the FE JS soon.

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked that into my agentDisconnect branch a little while ago.

reid added a commit to reid/yeti that referenced this pull request Jun 13, 2012
reid added a commit that referenced this pull request Oct 9, 2012
Move rendering of test results away from lib/cli.js
into a FeedbackLineReporter in lib/reporter/feedback-line.js.

The idea is to create a other reporters, e.g. JUnitReporter,
with the same interface.
reid added a commit that referenced this pull request Oct 12, 2012
 - Introduce JUnitReporter.
 - Introduce `--junit` and `--output junit` CLI options.
reid added a commit that referenced this pull request Oct 12, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants