Skip to content

Commit

Permalink
Don't catch exceptions thrown from onReceived handlers in JS
Browse files Browse the repository at this point in the history 
- We still catch these exceptions in the .NET client since there seems
  to be no better way to allow developers to observe exceptions thrown
  in the Received handler

#1530
  • Loading branch information
@halter73
halter73 committed Feb 18, 2013
1 parent 56a1cfe commit 58c9a6e
Show file tree
Hide file tree
Showing 13 changed files with 335 additions and 37 deletions.
8 changes: 1 addition & 7 deletions samples/Microsoft.AspNet.SignalR.Hosting.AspNet.Samples/Scripts/jquery.signalR.js
View file
Expand Up @@ -881,13 +881,7 @@

if (data.Messages) {
$.each(data.Messages, function () {
try {
$connection.triggerHandler(events.onReceived, [this]);
}
catch (e) {
connection.log("Error raising received " + e);
$(connection).triggerHandler(events.onError, [e]);
}
$connection.triggerHandler(events.onReceived, [this]);
});
}

Expand Down
2 changes: 1 addition & 1 deletion samples/Microsoft.AspNet.SignalR.Hosting.AspNet.Samples/Scripts/jquery.signalR.min.js
View file

Large diffs are not rendered by default.

8 changes: 1 addition & 7 deletions samples/Microsoft.AspNet.SignalR.LoadTestHarness/Scripts/jquery.signalR.js
View file
Expand Up @@ -881,13 +881,7 @@

if (data.Messages) {
$.each(data.Messages, function () {
try {
$connection.triggerHandler(events.onReceived, [this]);
}
catch (e) {
connection.log("Error raising received " + e);
$(connection).triggerHandler(events.onError, [e]);
}
$connection.triggerHandler(events.onReceived, [this]);
});
}

Expand Down
2 changes: 1 addition & 1 deletion samples/Microsoft.AspNet.SignalR.LoadTestHarness/Scripts/jquery.signalR.min.js
View file

Large diffs are not rendered by default.

8 changes: 1 addition & 7 deletions src/Microsoft.AspNet.SignalR.Client.JS/jquery.signalR.transports.common.js
View file
Expand Up @@ -230,13 +230,7 @@

if (data.Messages) {
$.each(data.Messages, function () {
try {
$connection.triggerHandler(events.onReceived, [this]);
}
catch (e) {
connection.log("Error raising received " + e);
$(connection).triggerHandler(events.onError, [e]);
}
$connection.triggerHandler(events.onReceived, [this]);
});
}

Expand Down
8 changes: 1 addition & 7 deletions tests/Microsoft.AspNet.SignalR.Client.JS.Tests/Scripts/jquery.signalR.js
View file
Expand Up @@ -881,13 +881,7 @@

if (data.Messages) {
$.each(data.Messages, function () {
try {
$connection.triggerHandler(events.onReceived, [this]);
}
catch (e) {
connection.log("Error raising received " + e);
$(connection).triggerHandler(events.onError, [e]);
}
$connection.triggerHandler(events.onReceived, [this]);
});
}

Expand Down
58 changes: 58 additions & 0 deletions tests/Microsoft.AspNet.SignalR.Client.JS.Tests/Tests/FunctionalTests/Hubs/HubProxyFacts.js
View file
Expand Up @@ -21,4 +21,62 @@ testUtilities.runWithTransports(["foreverFrame", "serverSentEvents", "webSockets
connection.stop();
};
});
});

testUtilities.runWithAllTransports(function (transport) {
QUnit.asyncTimeoutTest(transport + " transport does not capture exceptions thrown in invocation callbacks.", testUtilities.defaultTestTimeout, function (end, assert, testName) {
var connection = testUtilities.createHubConnection(testName),
demo = connection.createHubProxies().demo,
onerror = window.onerror;

window.onerror = function (errorMessage) {
assert.ok(errorMessage.match(/overload error/));
end();
return true;
}

connection.start({ transport: transport }).done(function () {
demo.server.overload().done(function () {
throw new Error("overload error");
});
}).fail(function (reason) {
assert.ok(false, "Failed to initiate SignalR connection");
end();
});

// Cleanup
return function () {
window.onerror = onerror;
connection.stop();
};
});

QUnit.asyncTimeoutTest(transport + " transport does not capture exceptions thrown in client hub methods.", testUtilities.defaultTestTimeout, function (end, assert, testName) {
var connection = testUtilities.createHubConnection(testName),
demo = connection.createHubProxies().demo,
onerror = window.onerror;

window.onerror = function (errorMessage) {
assert.ok(errorMessage.match(/error in callback/));
end();
return true;
}

demo.client.errorInCallback = function () {
throw new Error("error in callback");
}

connection.start({ transport: transport }).done(function () {
demo.server.doSomethingAndCallError();
}).fail(function (reason) {
assert.ok(false, "Failed to initiate SignalR connection");
end();
});

// Cleanup
return function () {
window.onerror = onerror;
connection.stop();
};
});
});
26 changes: 26 additions & 0 deletions ...ft.AspNet.SignalR.Client.JS.Tests/Tests/FunctionalTests/Transports/All/ConnectionFacts.js
View file
Expand Up @@ -60,4 +60,30 @@ testUtilities.runWithAllTransports(function (transport) {
connection.stop();
};
});

QUnit.asyncTimeoutTest(transport + " transport does not capture exceptions thrown in onReceived.", testUtilities.defaultTestTimeout, function (end, assert, testName) {
var connection = testUtilities.createConnection("multisend", testName),
onerror = window.onerror;

window.onerror = function (errorMessage) {
assert.ok(errorMessage.match(/onReceived error/));
end();
return true;
}

connection.received(function (data) {
throw new Error("onReceived error");
});

connection.start({ transport: transport }).fail(function (reason) {
assert.ok(false, "Failed to initiate SignalR connection");
end();
});

// Cleanup
return function () {
window.onerror = onerror;
connection.stop();
};
});
});
53 changes: 46 additions & 7 deletions ...s/Microsoft.AspNet.SignalR.Client.JS.Tests/Tests/UnitTests/Common/ProcessMessagesFacts.js
View file
@@ -1,7 +1,7 @@
QUnit.module("Transports Common - Process Messages Facts");

QUnit.test("Noop's on missing transport", function () {
var connection = testUtilities.createHubConnection(),
var connection = testUtilities.createConnection(),
lastKeepAlive = false;

// Ensure the connection can utilize the keep alive features
Expand All @@ -15,7 +15,7 @@ QUnit.test("Noop's on missing transport", function () {
});

QUnit.test("Updates keep alive data on any message retrieval.", function () {
var connection = testUtilities.createHubConnection(),
var connection = testUtilities.createConnection(),
response = {
C: 1234,
M: [{ uno: 1, dos: 2 }, { tres: 3, quatro: 4 }],
Expand Down Expand Up @@ -44,7 +44,7 @@ QUnit.test("Updates keep alive data on any message retrieval.", function () {
});

QUnit.test("Noop's on keep alive", function () {
var connection = testUtilities.createHubConnection(),
var connection = testUtilities.createConnection(),
savedMaximizePersistentResponse = $.signalR.transports._logic.maximizePersistentResponse,
failed = false;

Expand All @@ -65,7 +65,7 @@ QUnit.test("Noop's on keep alive", function () {
});

QUnit.test("Handles disconnect command correctly", function () {
var connection = testUtilities.createHubConnection(),
var connection = testUtilities.createConnection(),
response = {
C: 1234,
M: [{ uno: 1, dos: 2 }, { tres: 3, quatro: 4 }],
Expand All @@ -85,7 +85,7 @@ QUnit.test("Handles disconnect command correctly", function () {
});

QUnit.test("Updates group on message retrieval.", function () {
var connection = testUtilities.createHubConnection(),
var connection = testUtilities.createConnection(),
response = {
C: 1234,
M: [{ uno: 1, dos: 2 }, { tres: 3, quatro: 4 }],
Expand Down Expand Up @@ -115,7 +115,7 @@ QUnit.test("Updates group on message retrieval.", function () {
$.signalR.transports._logic.updateGroups = savedUpdateGroups;
});

QUnit.test("Triggeres received handler for each message.", function () {
QUnit.test("Triggers received handler for each message.", function () {
var connection = testUtilities.createHubConnection(),
demo = connection.createHubProxies().demo,
response = {
Expand All @@ -138,7 +138,7 @@ QUnit.test("Triggeres received handler for each message.", function () {
});

QUnit.test("Message ID is set on connection ID when set.", function () {
var connection = testUtilities.createHubConnection(),
var connection = testUtilities.createConnection(),
response = {
M: false,
L: 1337,
Expand All @@ -154,4 +154,43 @@ QUnit.test("Message ID is set on connection ID when set.", function () {
response.C = 1234;
$.signalR.transports._logic.processMessages(connection, response);
QUnit.ok(connection.messageId === 1234, "The connection's messageId property is set when a message contains a messageId");
});

QUnit.test("Exceptions thrown in onReceived handlers on not captured.", function () {
var connection = testUtilities.createConnection(),
hubConnection = testUtilities.createHubConnection(),
demo = hubConnection.createHubProxies().demo,
response = {
C: 1234,
M: [{ H: "demo", M: "foo", A: [], S: { value: 555 } }, { H: "demo", M: "foo", A: [], S: { value: 782 } }],
L: 1337,
G: "foo"
},
error = new Error(),
hubError = new Error();

connection.transport = {};
hubConnection.transport = {};

connection.received(function () {
throw error;
});
demo.client.foo = function () {
throw hubError;
};

try {
$.signalR.transports._logic.processMessages(connection, response);
QUnit.ok(false, "Error in onReceived handlers shouldn't be caught");
} catch (e) {
QUnit.equal(e, error);
}

$(hubConnection).triggerHandler($.signalR.events.onStarting);
try {
$.signalR.transports._logic.processMessages(hubConnection, response);
QUnit.ok(false, "Error in onReceived handlers shouldn't be caught");
} catch (e) {
QUnit.equal(e, hubError);
}
});
36 changes: 36 additions & 0 deletions tests/Microsoft.AspNet.SignalR.FunctionalTests/Client/HubProxyFacts.cs
View file
Expand Up @@ -89,6 +89,42 @@ public void UnableToCreateHubThrowsError(HostType hostType, TransportType transp
}
}

[Theory]
[InlineData(HostType.Memory, TransportType.ServerSentEvents)]
[InlineData(HostType.Memory, TransportType.LongPolling)]
[InlineData(HostType.IISExpress, TransportType.Websockets)]
public void ConnectionErrorCapturesExceptionsThrownInClientHubMethod(HostType hostType, TransportType transportType)
{
using (var host = CreateHost(hostType, transportType))
{
var wh = new ManualResetEventSlim();
Exception thrown = new Exception(),
caught = null;

host.Initialize();

var connection = CreateHubConnection(host);
var proxy = connection.CreateHubProxy("ChatHub");

proxy.On("addMessage", () =>
{
throw thrown;
});

connection.Error += e =>
{
caught = e;
wh.Set();
};

connection.Start(host.Transport).Wait();
proxy.Invoke("Send", "");

Assert.True(wh.Wait(TimeSpan.FromSeconds(5)));
Assert.Equal(thrown, caught);
}
}

public class MyHub2 : Hub
{
public MyHub2(int n)
Expand Down

0 comments on commit 58c9a6e

Please sign in to comment.