ChatSession.onConnectionEstablished() is fired twice after invoking ChatSession.connect()

[marcogrcr]

Steps to reproduce:

import "amazon-connect-chatjs"; // v1.3.1
import {
  ConnectClient,
  StartChatContactCommand,
} from "@aws-sdk/client-connect"; // v3.254.0

// start a chat contact
const client = new ConnectClient({
  region: "us-east-1",
  credentials: {
    accessKeyId: "...",
    secretAccessKey: "...",
    sessionToken: "...",
  },
});

const { ContactId, ParticipantId, ParticipantToken } = await client.send(
  new StartChatContactCommand({
    InstanceId: "...",
    ContactFlowId: "...",
    ParticipantDetails: { DisplayName: "Customer" },
  })
);

// create chat session
const session = connect.ChatSession.create({
  chatDetails: {
    contactId: ContactId,
    participantId: ParticipantId,
    participantToken: ParticipantToken,
  },
  options: { region: "us-east-1" },
  type: connect.ChatSession.SessionTypes.CUSTOMER,
});

// subscribe to `onConnectionEstablished` events
session.onConnectionEstablished((event) => {
  console.log("Connection established:", event);
});

// connect to chat
await session.connect();

Expected result:

The onConnectionEstablished listener gets invoked once:

Connection established: {
  chatDetails: { ... },
  data: {}
}

Actual result:

The onConnectionEstablished listener gets invoked twice with different event payloads:

Connection established: {
  chatDetails: { ... },
  connectCalled: true,
  connectSuccess: true,
  metadata: null,
  _debug: { webSocketStatus: "Starting" }
}

Connection established: {
  chatDetails: { ... },
  data: {}
}

Analysis:

This occurs because ChatController actually dispatches the event twice from two different places:

1. As part of the ChatController.prototype.connect() call:

amazon-connect-chatjs/src/core/chatController.js

Lines 192 to 201 in d85c517

	connect(args={}) {
	this.sessionMetadata = args.metadata || null;
	this.argsValidator.validateConnectChat(args);
	const connectionDetailsProvider = this._getConnectionDetailsProvider();
	return connectionDetailsProvider.fetchConnectionDetails()
	.then(
	(connectionDetails) =>
	this._initConnectionHelper(connectionDetailsProvider, connectionDetails)
	)
	.then(response => this._onConnectSuccess(response, connectionDetailsProvider))

amazon-connect-chatjs/src/core/chatController.js

Lines 292 to 304 in d85c517

	_onConnectSuccess(response, connectionDetailsProvider) {
	this._sendInternalLogToServer(this.logger.info("Connect successful!"));
	console.warn("onConnectionSuccess response", response);
	const responseObject = {
	_debug: response,
	connectSuccess: true,
	connectCalled: true,
	metadata: this.sessionMetadata
	};
	const eventData = Object.assign({
	chatDetails: this.getChatDetails()
	}, responseObject);
	this.pubsub.triggerAsync(CHAT_EVENTS.CONNECTION_ESTABLISHED, eventData);

2. Via an event listener of LpcConnectionHelper.prototype.handleConnectionGain():

amazon-connect-chatjs/src/core/chatController.js

Lines 192 to 199 in d85c517

	connect(args={}) {
	this.sessionMetadata = args.metadata || null;
	this.argsValidator.validateConnectChat(args);
	const connectionDetailsProvider = this._getConnectionDetailsProvider();
	return connectionDetailsProvider.fetchConnectionDetails()
	.then(
	(connectionDetails) =>
	this._initConnectionHelper(connectionDetailsProvider, connectionDetails)

amazon-connect-chatjs/src/core/chatController.js

Lines 207 to 218 in d85c517

	_initConnectionHelper(connectionDetailsProvider, connectionDetails) {
	this.connectionHelper = new LpcConnectionHelper(
	this.contactId,
	this.initialContactId,
	connectionDetailsProvider,
	this.websocketManager,
	this.logMetaData,
	connectionDetails
	);
	this.connectionHelper.onEnded(this._handleEndedConnection.bind(this));
	this.connectionHelper.onConnectionLost(this._handleLostConnection.bind(this));
	this.connectionHelper.onConnectionGain(this._handleGainedConnection.bind(this));

amazon-connect-chatjs/src/core/chatController.js

Lines 247 to 252 in d85c517

	_handleGainedConnection(eventData) {
	this._forwardChatEvent(CHAT_EVENTS.CONNECTION_ESTABLISHED, {
	data: eventData,
	chatDetails: this.getChatDetails()
	});
	}

amazon-connect-chatjs/src/core/chatController.js

Lines 288 to 290 in d85c517

	_forwardChatEvent(eventName, eventData) {
	this.pubsub.triggerAsync(eventName, eventData);
	}

Having duplicate event invocations can be problematic when the user assumes it will be invoked only once and the handler implementation is not idempotent (i.e. invoking it again may have undesired side-effects).

Proposed fix:

The event dispatch that is part of ChatController.prototype.connect() should be removed, so that only the LpcConnectionHelper.prototype.handleConnectionGain() remains. This is because the latter event will be triggered whenever the connection is lost and later regained.

[sseidel16]

The code fix does not fix the described issue of CHAT_EVENTS.CONNECTION_ESTABLISHED being fired twice. There is still an event fired from the connect call, and one from LpcConnectionHelper. Perhaps this is desired behavior, but that should be noted. As it stands, onConnectionEstablished is still called twice when connect is called a single time.

This is particularly interesting because when using amazon-connect-chat-interface, the duplicated call to onConnectionEstablished causes the transcript API to be called twice, which could seriously affect rate limits.

[marcogrcr]

@sseidel16 You are correct. I believe this may have been the result of a mixup. Looks like PR #196 resolved #123 instead of this issue.

[sseidel16]

Another interesting note:

The proposed fix in the initial issue is likely not feasible because, when used in conjunction with streams, the websocket can be already connected when connect is called (wsm has already connected). In that scenario, onConnectionEstablished is only called by connect, and therefore only invoked once. It seems more reasonable to only emit a connection established event after connect if the websocket has already been started (this can be seen in the response from LpcConnectionHelper start).