Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Commit

Permalink
Removing transport fallback in the client
Browse files Browse the repository at this point in the history
  • Loading branch information
moozzyk committed Nov 14, 2016
1 parent 97bf8c3 commit 325c909
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 58 deletions.
9 changes: 3 additions & 6 deletions samples/SocketsSample/wwwroot/hubs.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,9 @@
}

document.addEventListener('DOMContentLoaded', () => {
var transports = getParameterByName('transport');
if (transports != null) {
transports = transports.split(',')
}
let transport = getParameterByName('transport') || 'webSockets';

document.getElementById('head1').innerHTML = transports ? transports.join(', ') : "auto (WebSockets)";
document.getElementById('head1').innerHTML = transport;

let connection = new RpcConnection(`http://${document.location.host}/hubs`, 'formatType=json&format=text');
connection.on('Send', msg => {
Expand All @@ -70,7 +67,7 @@
}

click('connect', event => {
connection.start(transports)
connection.start(transport)
.then(() => {
isConnected = true;
addLine('Connected successfully', 'green');
Expand Down
69 changes: 19 additions & 50 deletions src/Microsoft.AspNetCore.SignalR.Client.TS/Connection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,76 +20,44 @@ class Connection {
this.connectionState = ConnectionState.Disconnected;
}

start(transportNames?: string[]): Promise<void> {
start(transportName: string = 'webSockets'): Promise<void> {
if (this.connectionState != ConnectionState.Disconnected) {
throw new Error("Cannot start a connection that is not in the 'Disconnected' state");
}

let transports = this.filterTransports(transportNames);
if (transports.length == 0) {
throw new Error("No valid transports requested.");
}
this.transport = this.createTransport(transportName);
this.transport.onDataReceived = this.dataReceivedCallback;
this.transport.onError = e => this.stopConnection();

return new HttpClient().get(`${this.url}/getid?${this.queryString}`)
.then(connectionId => {
this.connectionId = connectionId;
this.queryString = `id=${connectionId}&${this.connectionId}`;
return this.tryStartTransport(transports, 0);
return this.transport.connect(this.url, this.queryString);
})
.then(transport => {
this.transport = transport;
.then(() => {
this.connectionState = ConnectionState.Connected;
})
.catch(e => {
console.log("Failed to start the connection.")
this.connectionState = ConnectionState.Disconnected;
this.transport = null;
throw e;
});
}

private filterTransports(transportNames: string[]): ITransport[] {
let availableTransports = ['webSockets', 'serverSentEvents', 'longPolling'];
transportNames = transportNames || availableTransports;
// uniquify
transportNames = transportNames.filter((value, index, values) => {
return values.indexOf(value) == index;
});

let transports: ITransport[] = [];
transportNames.forEach(transportName => {
if (transportName === 'webSockets') {
transports.push(new WebSocketTransport());
}
if (transportName === 'serverSentEvents') {
transports.push(new ServerSentEventsTransport());
}
if (transportName === 'longPolling') {
transports.push(new LongPollingTransport());
}
});

return transports;
}

private tryStartTransport(transports: ITransport[], index: number): Promise<ITransport> {
let thisConnection = this;
transports[index].onDataReceived = data => thisConnection.dataReceivedCallback(data);
transports[index].onError = e => thisConnection.stopConnection(e);
private createTransport(transportName: string): ITransport {
if (transportName === 'webSockets') {
return new WebSocketTransport();
}
if (transportName === 'serverSentEvents') {
return new ServerSentEventsTransport();
}
if (transportName === 'longPolling') {
return new LongPollingTransport();
}

return transports[index].connect(this.url, this.queryString)
.then(() => {
return transports[index];
})
.catch(e => {
index++;
if (index < transports.length) {
return this.tryStartTransport(transports, index);
}
else
{
throw new Error('No transport could be started.')
}
})
throw new Error("No valid transports requested.");
}

send(data: any): Promise<void> {
Expand All @@ -109,6 +77,7 @@ class Connection {

private stopConnection(error?: any) {
this.transport.stop();
this.transport = null;
this.connectionState = ConnectionState.Disconnected;
this.connectionClosedCallback(error);
}
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.AspNetCore.SignalR.Client.TS/RpcConnection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ class RpcConnection {
}
}

start(transportNames?:string[]): Promise<void> {
return this.connection.start(transportNames);
start(transportName? :string): Promise<void> {
return this.connection.start(transportName);
}

stop(): void {
Expand Down

9 comments on commit 325c909

@Marcelh1983
Copy link

Choose a reason for hiding this comment

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

@moozzyk I want to use websockets, but if it fails, at my company the proxy forbids websockets to azure.. it should fallback to long-polling. Should I build this myself? Try to connect websockets and on failure start connection using longpolling? Sorry for asking but couldn't find it anywhere.

@moozzyk
Copy link
Contributor

@moozzyk moozzyk commented on 325c909 Sep 7, 2017

Choose a reason for hiding this comment

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

@Marcelh1983 - yes - there is no built-in fallback, so you will have to do it yourself. I am thinking about creating a sample somewhere. If this is always going to fail you could disable the transport on the server and the client will pick next available one.

@Marcelh1983
Copy link

Choose a reason for hiding this comment

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

With always going to fail you mean all connecting clients right? For most clients websockets will work fine, just need a long polling fallback. Will build it :-). Thanks for the reply!

@moozzyk
Copy link
Contributor

@moozzyk moozzyk commented on 325c909 Sep 7, 2017

Choose a reason for hiding this comment

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

Yes, this is what I meant. If all clients would have to always go through a proxy that rejects websockets it might be just easier to disable websockets on the server. When doing fallback make sure you test on Edge/IE because of this.

@Marcelh1983
Copy link

Choose a reason for hiding this comment

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

Thanks for the reply. I wrote some code that tries to connect and return a promise with the connection on success and an empty promise on fail and tries all different protocols until it has a connection. I think I wrote it ok, but an example would be great because I think lots of people want a fallback protocol.

There are still some issues I experience, I will look into them and maybe the are because its just because its just production ready but maybe you can address these issues immediately.

  • Internet explorer 11 works fine on websockets. On long polling it connects fine, but as soon as it get the first data from the hub, it keeps sending infinit connecting requests.
  • On azure, while I turned on websockets it keeps connecting using long polling.

@davidfowl
Copy link
Member

Choose a reason for hiding this comment

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

Put the code in a GitHub repository and we'll take a look.

@moozzyk
Copy link
Contributor

Choose a reason for hiding this comment

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

Internet explorer 11 works fine on websockets. On long polling it connects fine, but as soon as it get the first data from the hub, it keeps sending infinit connecting requests.

Do you have the latest version of the client? This was fixed here: 2a36aa1#diff-ce36dc651371b453b15f91d17a548293R252
The issue is that IE caches responses for xhr requests. Adding the Cache-Control header fixes it for IE11. IE10 and IE9 apparently ignore this header. If people start hitting the issue on IE9/IE10 we can fix it by randomizing the url (this is what ajax from jQuery does).

@Marcelh1983
Copy link

Choose a reason for hiding this comment

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

I'm using the latest version. I added all ts files in my project and compiled it to es5. I also tried to use the es5 files in the npm package but same result.

I created a repository with a subset of our project, with the fallback (I commented out to test the IE longpolling error). If you start the backend and frontend you'll see IE keep sending connection requests.

Other issue that if I enable my fallback method it still uses longpolling on Azure while it uses Websockets locally. Will deploy this example on Azure tomorrow.

https://github.com/Marcelh1983/signalr-example
signalR service: https://github.com/Marcelh1983/signalr-example/blob/master/Frontend/src/app/shared/services/signalr.service.ts

@moozzyk
Copy link
Contributor

Choose a reason for hiding this comment

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

I opened #848 for this. Let's use it for further discussion instead of this almost a year old PR.

Please sign in to comment.