From ada9efd825fa683f2cf708aee086f8f20b6b8cf9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Casper=20K=C3=BCthe?=
 <43839798+Casper64@users.noreply.github.com>
Date: Fri, 22 Dec 2023 08:53:27 +0100
Subject: [PATCH] x.vweb: fix fsanitize-address test for SSE, improve
 documentation on the usage of `takeover_conn` (#20249)

---
 vlib/x/vweb/README.md      | 15 ++++++++++++++-
 vlib/x/vweb/context.v      |  3 +--
 vlib/x/vweb/sse/README.md  | 12 +++++++-----
 vlib/x/vweb/sse/sse.v      |  1 -
 vlib/x/vweb/sse/sse_test.v |  4 ++--
 vlib/x/vweb/vweb.v         |  6 ++++++
 6 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/vlib/x/vweb/README.md b/vlib/x/vweb/README.md
index 11cba4fb21..6a12591423 100644
--- a/vlib/x/vweb/README.md
+++ b/vlib/x/vweb/README.md
@@ -805,6 +805,16 @@ When this function is called you are free to do anything you want with the TCP
 connection and vweb will not interfere. This means that we are responsible for
 sending a response over the connection and closing it.
 
+### Empty Result
+
+Sometimes you want to send the response in another thread, for example when using 
+[Server Sent Events](sse/README.md). When you are sure that a response will be sent
+over the TCP connection you can return `vweb.no_result()`. This function does nothinng
+and returns an empty `vweb.Result` struct, letting vweb know that we sent a response ourself.
+
+> **Note:**
+> It is important to call `ctx.takeover_conn` before you spawn a thread
+
 **Example:**
 ```v
 module main
@@ -825,11 +835,14 @@ pub fn (app &App) index(mut ctx Context) vweb.Result {
 
 @['/long']
 pub fn (app &App) long_response(mut ctx Context) vweb.Result {
+	// let vweb know that the connection should not be closed
+	ctx.takeover_conn()
 	// use spawn to handle the connection in another thread
 	// if we don't the whole web server will block for 10 seconds,
 	// since vweb is singlethreaded
 	spawn handle_connection(mut ctx.conn)
-	return ctx.takeover_conn()
+	// we will send a custom response ourself, so we can safely return an empty result
+	return vweb.no_result()
 }
 
 fn handle_connection(mut conn net.TcpConn) {
diff --git a/vlib/x/vweb/context.v b/vlib/x/vweb/context.v
index 236a76e58b..f4c44f20cb 100644
--- a/vlib/x/vweb/context.v
+++ b/vlib/x/vweb/context.v
@@ -276,9 +276,8 @@ pub fn (mut ctx Context) set_content_type(mime string) {
 // send over the connetion and you can send multiple responses.
 // This function is usefull when you want to keep the connection alive and/or
 // send multiple responses. Like with the SSE.
-pub fn (mut ctx Context) takeover_conn() Result {
+pub fn (mut ctx Context) takeover_conn() {
 	ctx.takeover = true
-	return Result{}
 }
 
 // user_agent returns the user-agent header for the current client
diff --git a/vlib/x/vweb/sse/README.md b/vlib/x/vweb/sse/README.md
index 72b8ec6bd7..01bcaa17f9 100644
--- a/vlib/x/vweb/sse/README.md
+++ b/vlib/x/vweb/sse/README.md
@@ -11,9 +11,9 @@ With SSE we want to keep the connection open, so we are able to
 keep sending events to the client. But if we hold the connection open indefinitely
 vweb isn't able to process any other requests. 
 
-We can let vweb know that it can continue
-processing other requests and that we will handle the connection ourself by
-returning `ctx.takeover_conn()`. Vweb will not close the connection and we can handle
+We can let vweb know that it can continue processing other requests and that we will 
+handle the connection ourself by calling `ctx.takeover_conn()` and and returning an empty result
+with `vweb.no_result()`. Vweb will not close the connection and we can handle
 the connection in a seperate thread.
 
 **Example:**
@@ -22,10 +22,12 @@ import x.vweb.sse
 
 // endpoint handler for SSE connections
 fn (app &App) sse(mut ctx Context) vweb.Result {
+	// let vweb know that the connection should not be closed
+	ctx.takeover_conn()
 	// handle the connection in a new thread
 	spawn handle_sse_conn(mut ctx)
-	// let vweb know that the connection should not be closed
-	return ctx.takeover_conn()
+	// we will send a custom response ourself, so we can safely return an empty result
+	return vweb.no_result()
 }
 
 fn handle_sse_conn(mut ctx Context) {
diff --git a/vlib/x/vweb/sse/sse.v b/vlib/x/vweb/sse/sse.v
index d4f815b356..0140a3eaec 100644
--- a/vlib/x/vweb/sse/sse.v
+++ b/vlib/x/vweb/sse/sse.v
@@ -37,7 +37,6 @@ pub mut:
 
 // start an SSE connection
 pub fn start_connection(mut ctx vweb.Context) &SSEConnection {
-	ctx.takeover_conn()
 	ctx.res.header.set(.connection, 'keep-alive')
 	ctx.res.header.set(.cache_control, 'no-cache')
 	ctx.send_response_to_client('text/event-stream', '')
diff --git a/vlib/x/vweb/sse/sse_test.v b/vlib/x/vweb/sse/sse_test.v
index aa67e08098..3d88ebf785 100644
--- a/vlib/x/vweb/sse/sse_test.v
+++ b/vlib/x/vweb/sse/sse_test.v
@@ -14,12 +14,12 @@ pub struct Context {
 pub struct App {}
 
 fn (app &App) sse(mut ctx Context) vweb.Result {
+	ctx.takeover_conn()
 	spawn handle_sse_conn(mut ctx)
-	return ctx.takeover_conn()
+	return vweb.no_result()
 }
 
 fn handle_sse_conn(mut ctx Context) {
-	// pass vweb.Context
 	mut sse_conn := sse.start_connection(mut ctx.Context)
 
 	for _ in 0 .. 3 {
diff --git a/vlib/x/vweb/vweb.v b/vlib/x/vweb/vweb.v
index 935cc60d7f..15b4f954b0 100644
--- a/vlib/x/vweb/vweb.v
+++ b/vlib/x/vweb/vweb.v
@@ -20,6 +20,12 @@ pub type RawHtml = string
 @[noinit]
 pub struct Result {}
 
+// no_result does nothing, but returns `vweb.Result`. Only use it when you are sure
+// a response will be send over the connection, or in combination with `Context.takeover_conn`
+pub fn no_result() Result {
+	return Result{}
+}
+
 pub const methods_with_form = [http.Method.post, .put, .patch]
 
 pub const headers_close = http.new_custom_header_from_map({