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

vibe/http/server.d(875): AssertionError "A void body was already written!" #821

Closed
dcarp opened this Issue Sep 11, 2014 · 4 comments

Comments

Projects
None yet
2 participants
@dcarp
Contributor

dcarp commented Sep 11, 2014

This occurs because HTTPServerResponse.writeJsonBody must not be called more than once per instance.

Anyhow in the following code section (vibe/web/rest.d:540-561) HTTPServerResponse.writeJsonBody is called second time in the catch block, when an exception is thrown during the first call in the try block.

        try {
            import vibe.internal.meta.funcattr;

            auto handler = createAttributedFunction!Func(req, res);

            static if (is(RT == void)) {
                handler(&__traits(getMember, inst, method), params);
                res.writeJsonBody(Json.emptyObject);
            } else {
                auto ret = handler(&__traits(getMember, inst, method), params);
                res.writeJsonBody(ret);
            }
        } catch (HTTPStatusException e) {
            res.writeJsonBody([ "statusMessage": e.msg ], e.status);
        } catch (Exception e) {
            // TODO: better error description!
            logDebug("REST handler exception: %s", e.toString());
            res.writeJsonBody(
                [ "statusMessage": e.msg, "statusDebugMessage": sanitizeUTF8(cast(ubyte[])e.toString()) ],
                HTTPStatus.internalServerError
            );
        }
@dcarp

This comment has been minimized.

Show comment
Hide comment
@dcarp

dcarp Sep 11, 2014

Contributor

A solution could be to add a HTTPServerResponse.reset method, that resets the state of the object. The reset method will then be called in the catch block. I can produce a PR for such a solution.

Contributor

dcarp commented Sep 11, 2014

A solution could be to add a HTTPServerResponse.reset method, that resets the state of the object. The reset method will then be called in the catch block. I can produce a PR for such a solution.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Sep 12, 2014

Member

That would in general corrupt the HTTP protocol. The only safe solution would be to add an if (!res.headerWritten) and only write an error response at all when that passes.

Member

s-ludwig commented Sep 12, 2014

That would in general corrupt the HTTP protocol. The only safe solution would be to add an if (!res.headerWritten) and only write an error response at all when that passes.

@dcarp

This comment has been minimized.

Show comment
Hide comment
@dcarp

dcarp Sep 12, 2014

Contributor

Does res.headerWritten == true mean, that the header has already been sent over to the client?

What will happen if the suggested condition is added and res.headerWritten == true?

Contributor

dcarp commented Sep 12, 2014

Does res.headerWritten == true mean, that the header has already been sent over to the client?

What will happen if the suggested condition is added and res.headerWritten == true?

@dcarp

This comment has been minimized.

Show comment
Hide comment
@dcarp

dcarp Sep 12, 2014

Contributor

This is the exception thrown on the first call of HTTPServerResponse.writeJsonBody:

object.Exception@include/vibed/vibe/core/drivers/libevent2_tcp.d(367): Connection error while writing to TCPConnection.
----------------
./build/release/AnnouncementMonitor(void vibe.core.drivers.libevent2_tcp.Libevent2TCPConnection.write(const(ubyte[]))+0x70) [0x993844]
./build/release/AnnouncementMonitor(void vibe.core.stream.OutputStream.write(const(char[]))+0x5d) [0xa0e139]
./build/release/AnnouncementMonitor(void vibe.http.server.HTTPServerResponse.writeHeader()+0x50d) [0xbe8885]
./build/release/AnnouncementMonitor(@property vibe.core.stream.OutputStream vibe.http.server.HTTPServerResponse.bodyWriter()+0x6d8) [0xbe6098]
./build/release/AnnouncementMonitor(void vibe.http.server.HTTPServerResponse.writeJsonBody!(monitor.model.Announcement.Announcement[]).writeJsonBody(monitor.model.Announcement.Announcement[], int, immutable(char)[])+0x91) [0x74fd11]
./build/release/AnnouncementMonitor(void vibe.web.rest.__T17jsonMethodHandlerTC7monitor8protocol3Api3ApiVAyaa24_6765745370656369666963416e6e6f756e63656d656e7473S133_D7monitor8protocol3Api3Api24getSpecificAnnouncementsMFAyaAyaAyaAyaAyaAyaAyaAyaAyaAyaAyaZAS7monitor5model12Announcement12AnnouncementZ.jsonMethodHandler(monitor.protocol.Api.Api).handler(vibe.http.server.HTTPServerRequest, vibe.http.server.HTTPServerResponse)+0x1b2e) [0x75b4c6]
./build/release/AnnouncementMonitor(void vibe.http.router.URLRouter.handleRequest(vibe.http.server.HTTPServerRequest, vibe.http.server.HTTPServerResponse).void __lambda3!(ulong, immutable(char)[][]).__lambda3(ulong, scope immutable(char)[][])+0x275) [0xbb7ddd]
./build/release/AnnouncementMonitor(void vibe.http.router.MatchTree!(vibe.http.router.Route).MatchTree.match(immutable(char)[], scope void delegate(ulong, scope immutable(char)[][]))+0x175) [0xbb83bd]
./build/release/AnnouncementMonitor(void vibe.http.router.URLRouter.handleRequest(vibe.http.server.HTTPServerRequest, vibe.http.server.HTTPServerResponse)+0x1b7) [0xbb7aff]
./build/release/AnnouncementMonitor(bool vibe.http.server.handleRequest(vibe.core.stream.Stream, vibe.core.net.TCPConnection, vibe.http.server.HTTPServerListener, ref vibe.http.server.HTTPServerSettings, ref bool)+0x3b87) [0xbef257]
./build/release/AnnouncementMonitor(void vibe.http.server.handleHTTPConnection(vibe.core.net.TCPConnection, vibe.http.server.HTTPServerListener)+0x23b) [0xbeb48b]
./build/release/AnnouncementMonitor(void vibe.http.server.listenHTTPPlain(vibe.http.server.HTTPServerSettings).doListen(vibe.http.server.HTTPServerSettings, vibe.http.server.HTTPServerListener, immutable(char)[]).__lambda4(vibe.core.net.TCPConnection)+0x28) [0xbe3350]
./build/release/AnnouncementMonitor(void vibe.core.drivers.libevent2_tcp.onConnect(int, short, void*).ClientTask.execute()+0x5b0) [0x9951ac]
./build/release/AnnouncementMonitor(_D4vibe4core4core12__T7runTaskZ7runTaskFDFZvZS4vibe4core4task4Task12callDelegateFC4vibe4core4core8CoreTaskZv+0x24) [0x7115bc]
./build/release/AnnouncementMonitor(void vibe.core.core.CoreTask.run()+0x2a4) [0x91d6fc]
./build/release/AnnouncementMonitor(void core.thread.Fiber.run()+0x2a) [0x102b28a]
./build/release/AnnouncementMonitor(fiber_entryPoint+0x61) [0x102b195]
[(nil)]

For this case, it will be OK to just log the error and give-up the HTTPServerResponse processing.

The clean solution would be that at vibe/core/drivers/libevent2_tcp.d(367) and co., instead of the generic Exception, a specialized ConnectionException is thrown. In this way, when you catch ConnectionException, you don't even try to call HTTPServerResponse.writeJsonBody anymore.

Contributor

dcarp commented Sep 12, 2014

This is the exception thrown on the first call of HTTPServerResponse.writeJsonBody:

object.Exception@include/vibed/vibe/core/drivers/libevent2_tcp.d(367): Connection error while writing to TCPConnection.
----------------
./build/release/AnnouncementMonitor(void vibe.core.drivers.libevent2_tcp.Libevent2TCPConnection.write(const(ubyte[]))+0x70) [0x993844]
./build/release/AnnouncementMonitor(void vibe.core.stream.OutputStream.write(const(char[]))+0x5d) [0xa0e139]
./build/release/AnnouncementMonitor(void vibe.http.server.HTTPServerResponse.writeHeader()+0x50d) [0xbe8885]
./build/release/AnnouncementMonitor(@property vibe.core.stream.OutputStream vibe.http.server.HTTPServerResponse.bodyWriter()+0x6d8) [0xbe6098]
./build/release/AnnouncementMonitor(void vibe.http.server.HTTPServerResponse.writeJsonBody!(monitor.model.Announcement.Announcement[]).writeJsonBody(monitor.model.Announcement.Announcement[], int, immutable(char)[])+0x91) [0x74fd11]
./build/release/AnnouncementMonitor(void vibe.web.rest.__T17jsonMethodHandlerTC7monitor8protocol3Api3ApiVAyaa24_6765745370656369666963416e6e6f756e63656d656e7473S133_D7monitor8protocol3Api3Api24getSpecificAnnouncementsMFAyaAyaAyaAyaAyaAyaAyaAyaAyaAyaAyaZAS7monitor5model12Announcement12AnnouncementZ.jsonMethodHandler(monitor.protocol.Api.Api).handler(vibe.http.server.HTTPServerRequest, vibe.http.server.HTTPServerResponse)+0x1b2e) [0x75b4c6]
./build/release/AnnouncementMonitor(void vibe.http.router.URLRouter.handleRequest(vibe.http.server.HTTPServerRequest, vibe.http.server.HTTPServerResponse).void __lambda3!(ulong, immutable(char)[][]).__lambda3(ulong, scope immutable(char)[][])+0x275) [0xbb7ddd]
./build/release/AnnouncementMonitor(void vibe.http.router.MatchTree!(vibe.http.router.Route).MatchTree.match(immutable(char)[], scope void delegate(ulong, scope immutable(char)[][]))+0x175) [0xbb83bd]
./build/release/AnnouncementMonitor(void vibe.http.router.URLRouter.handleRequest(vibe.http.server.HTTPServerRequest, vibe.http.server.HTTPServerResponse)+0x1b7) [0xbb7aff]
./build/release/AnnouncementMonitor(bool vibe.http.server.handleRequest(vibe.core.stream.Stream, vibe.core.net.TCPConnection, vibe.http.server.HTTPServerListener, ref vibe.http.server.HTTPServerSettings, ref bool)+0x3b87) [0xbef257]
./build/release/AnnouncementMonitor(void vibe.http.server.handleHTTPConnection(vibe.core.net.TCPConnection, vibe.http.server.HTTPServerListener)+0x23b) [0xbeb48b]
./build/release/AnnouncementMonitor(void vibe.http.server.listenHTTPPlain(vibe.http.server.HTTPServerSettings).doListen(vibe.http.server.HTTPServerSettings, vibe.http.server.HTTPServerListener, immutable(char)[]).__lambda4(vibe.core.net.TCPConnection)+0x28) [0xbe3350]
./build/release/AnnouncementMonitor(void vibe.core.drivers.libevent2_tcp.onConnect(int, short, void*).ClientTask.execute()+0x5b0) [0x9951ac]
./build/release/AnnouncementMonitor(_D4vibe4core4core12__T7runTaskZ7runTaskFDFZvZS4vibe4core4task4Task12callDelegateFC4vibe4core4core8CoreTaskZv+0x24) [0x7115bc]
./build/release/AnnouncementMonitor(void vibe.core.core.CoreTask.run()+0x2a4) [0x91d6fc]
./build/release/AnnouncementMonitor(void core.thread.Fiber.run()+0x2a) [0x102b28a]
./build/release/AnnouncementMonitor(fiber_entryPoint+0x61) [0x102b195]
[(nil)]

For this case, it will be OK to just log the error and give-up the HTTPServerResponse processing.

The clean solution would be that at vibe/core/drivers/libevent2_tcp.d(367) and co., instead of the generic Exception, a specialized ConnectionException is thrown. In this way, when you catch ConnectionException, you don't even try to call HTTPServerResponse.writeJsonBody anymore.

@s-ludwig s-ludwig closed this in 00caa4d Sep 12, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment