Skip to content

Commit

Permalink
Do not associate a heartbeat with a websocket message
Browse files Browse the repository at this point in the history
  • Loading branch information
@jfarcand
jfarcand committed May 27, 2014
1 parent 2508b92 commit a015c69
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions modules/cpr/src/main/java/org/atmosphere/interceptor/HeartbeatInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ public Clock() {
@Override
public Action inspect(final AtmosphereResource r) {

if (Utils.webSocketMessage(r)) return Action.CONTINUE;

final int interval = extractHeartbeatInterval(r);

if (interval != 0) {
Expand Down

3 comments on commit a015c69

@gdrouet
Copy link
Member

@gdrouet gdrouet commented on a015c69 Jun 8, 2014

Choose a reason for hiding this comment

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

@jfarcand this breaks detection when client sends heartbeat: https://github.com/Atmosphere/atmosphere-javascript/blob/master/modules/javascript/src/main/webapp/javascript/atmosphere.js#L1493

Didn't really get what's the logic of 'Utils.webSocketMessage(r)' (which return true when heartbeat is sent). Can you help fix this?

@jfarcand
Copy link
Member Author

Choose a reason for hiding this comment

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

Utils.webSocketMessage(r) is used to avoid adding listerners to the AtmosphereResource associated with every websocket message. So what needs to be done here is if the client's heartbeat is enabled, then this condition should be skipped. If the client doesn't send heartbeat, then we need to leave the code as it is so we no listener ot processing are done on the AtmosphereResource. Does it help?

@gdrouet
Copy link
Member

@gdrouet gdrouet commented on a015c69 Jun 9, 2014

Choose a reason for hiding this comment

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

Ok perfect I understand :-)

Please sign in to comment.