Skip to content
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

Exception gets swallowed #1421

Open
John-Colvin opened this issue Feb 12, 2016 · 3 comments
Open

Exception gets swallowed #1421

John-Colvin opened this issue Feb 12, 2016 · 3 comments

Comments

@John-Colvin
Copy link
Contributor

I modified examples/websocket to throw an exception on connection, like this:

import vibe.core.log;
import vibe.http.fileserver : serveStaticFiles;
import vibe.http.router : URLRouter;
import vibe.http.server;
import vibe.http.websockets : WebSocket, handleWebSockets;

shared static this()
{
    auto router = new URLRouter;
    router.get("/ws", handleWebSockets(&handleWebSocketConnection));

    auto settings = new HTTPServerSettings;
    settings.port = 8080;
    settings.bindAddresses = ["::1", "127.0.0.1"];
    listenHTTP(settings, router);
}

void handleWebSocketConnection(scope WebSocket socket)
{
    logInfo("hello");
    throw new Exception("");
}

When I connect to the socket, hello is printed, but there is no sign of the exception and the program continues uninterrupted.

@Laeeth
Copy link

Laeeth commented Feb 15, 2016

John - do you haven impression this used to work with older vibed and current behaviour is a regression ? Or was it always like that ?

@jdeschenes
Copy link

@John-Colvin

I modified your example code slightly to log the the information into a file by adding this line:

setLogFile("websocket.log", LogLevel.debug_);

I saw that the exception was in fact getting logged into the file.

The code in question

Once the error is caught, a message of LogLevel.diagnostic is emitted but has a lower level than LogLevel.info and thus doesn't get printed by default.

I personally think this is an undesired behavior. An uncaught exception should:

  1. emit a message of LogLevel.error that provides the error message with the traceback information
  2. Close the socket that caused the error
  3. Resume normal operation (the catch in that case is entirely correct)

So really, changing the line to this would solve the issue:

logError("WebSocket handler failed: %s", e);

I will create a pull request in a few.

@John-Colvin
Copy link
Contributor Author

@jdeschenes Thank's for looking in to this. I currently work around it by catching the exception myself, printing it, then rethrowing, but it would be much better to have it correct by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants