Skip to content

fix: socket not cleared on time for second sync #3783

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

Merged
merged 1 commit into from
Aug 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/definitions/livesync.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -452,8 +452,9 @@ interface IAndroidLivesyncTool {

/**
* Closes the current socket connection.
* @param error - Optional error for rejecting pending sync operations
*/
end(): void;
end(error?: Error): void;
}

interface IAndroidLivesyncToolConfiguration {
Expand Down
3 changes: 2 additions & 1 deletion lib/services/livesync/android-livesync-tool.md
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,9 @@ End will close the current liveSync socket. Any sync operations that are still i
```TypeScript
/**
* Closes the current socket connection.
* @param error - Optional error for rejecting pending sync operations
*/
end(): void;
end(error? Error): void;
```

* Example:
Expand Down
27 changes: 22 additions & 5 deletions lib/services/livesync/android-livesync-tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,17 @@ export class AndroidLivesyncTool implements IAndroidLivesyncTool {
return operationPromise;
}

public end() {
public end(error?: Error) {
if (this.socketConnection) {
this.socketConnection.end();
const socketUid = this.socketConnection.uid;
const socket = this.socketConnection;
error = error || this.getErrorWithMessage("Socket connection ended before sync operation is complete.");
//remove listeners and delete this.socketConnection
this.cleanState(socketUid);
//call end of the connection (close and error callbacks won't be called - listeners removed)
socket.end();
//reject all pending sync requests and clear timeouts
this.rejectPendingSyncOperations(socketUid, error);
}
}

Expand Down Expand Up @@ -381,12 +389,21 @@ export class AndroidLivesyncTool implements IAndroidLivesyncTool {
private handleSocketError(socketId: string, errorMessage: string) {
const error = this.getErrorWithMessage(errorMessage);
if (this.socketConnection && this.socketConnection.uid === socketId) {
this.end();
this.socketError = error;
this.end(error);
Copy link
Contributor

@Fatme Fatme Aug 1, 2018

Choose a reason for hiding this comment

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

Why do we need to pass error here as parameter? We can use it directly in end() function

public end() {
const error = this.socketError || this.getErrorWithMessage()
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a personal preference. If someone moves this.socketError = error one line down or the error is not cleared at the right moment, the method might be broken without us noticing.

} else {
this.rejectPendingSyncOperations(socketId, error);
}
}

private cleanState(socketId: string) {
if (this.socketConnection && this.socketConnection.uid === socketId) {
this.socketConnection.removeAllListeners();
this.socketConnection = null;
this.socketError = error;
}
}

private rejectPendingSyncOperations(socketId: string, error: Error) {
_.keys(this.operationPromises)
.forEach(operationId => {
const operationPromise = this.operationPromises[operationId];
Expand All @@ -395,7 +412,7 @@ export class AndroidLivesyncTool implements IAndroidLivesyncTool {
operationPromise.reject(error);
delete this.operationPromises[operationId];
}
});
});
}

private getErrorWithMessage(errorMessage: string) {
Expand Down
Empty file.