noodlefrenzy / node-amqp10

amqp10 is a promise-based, AMQP 1.0 compliant node.js client
MIT License
134 stars 56 forks source link

fix(session): disable reestablish on disconnect and guard against duplicate begin() calls #353

Closed princjef closed 6 years ago

princjef commented 6 years ago

With reestablish enabled for sessions, the session can end up in a state where it is trying to reestablish itself while the connection is trying to do the same, leading to duplicate calls to begin(). The following has been changed to better handle this situation and others like it:


This change is Reviewable

pierreca commented 6 years ago

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.


lib/session.js, line 181 at r1 (raw file):

  var state = this.sm.getMachineState();
  debug('trying to begin() a session in state ' + state);
  switch (state) {

wouldn't that switch mean that we need to make "begin" an action of the state machine?


Comments from Reviewable

princjef commented 6 years ago

lib/session.js, line 181 at r1 (raw file):

Previously, pierreca (Pierre Cauchois) wrote…
wouldn't that switch mean that we need to make "begin" an action of the state machine?

good call. moved begin() into the state machine in place of beginSent() with the appropriate guards/checks


Comments from Reviewable

pierreca commented 6 years ago
:lgtm:

Reviewed 1 of 1 files at r1, 2 of 2 files at r2. Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable