Skip to content

Commit

Permalink
feat(Bun.serve idleTimeout) allow custom timeouts (#13453)
Browse files Browse the repository at this point in the history
  • Loading branch information
cirospaciari authored Aug 22, 2024
1 parent fe62a61 commit 384988f
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 31 deletions.
8 changes: 8 additions & 0 deletions packages/bun-types/bun.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2342,6 +2342,14 @@ declare module "bun" {
*/
unix?: never;

/**
* Sets the the number of seconds to wait before timing out a connection
* due to inactivity.
*
* Default is `10` seconds.
*/
idleTimeout?: number;

/**
* Handle HTTP requests
*
Expand Down
18 changes: 8 additions & 10 deletions packages/bun-uws/src/HttpContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ struct HttpContext {
}

/* Any connected socket should timeout until it has a request */
us_socket_timeout(SSL, s, HTTP_IDLE_TIMEOUT_S);
((HttpResponse<SSL> *) s)->resetTimeout();

/* Call filter */
for (auto &f : httpContextData->filterHandlers) {
Expand All @@ -94,11 +94,10 @@ struct HttpContext {

/* Handle socket connections */
us_socket_context_on_open(SSL, getSocketContext(), [](us_socket_t *s, int /*is_client*/, char */*ip*/, int /*ip_length*/) {
/* Any connected socket should timeout until it has a request */
us_socket_timeout(SSL, s, HTTP_IDLE_TIMEOUT_S);

/* Init socket ext */
new (us_socket_ext(SSL, s)) HttpResponseData<SSL>;
/* Any connected socket should timeout until it has a request */
((HttpResponse<SSL> *) s)->resetTimeout();

if(!SSL) {
/* Call filter */
Expand Down Expand Up @@ -237,7 +236,7 @@ struct HttpContext {

/* If we have not responded and we have a data handler, we need to timeout to enfore client sending the data */
if (!((HttpResponse<SSL> *) s)->hasResponded() && httpResponseData->inStream) {
us_socket_timeout(SSL, (us_socket_t *) s, HTTP_IDLE_TIMEOUT_S);
((HttpResponse<SSL> *) s)->resetTimeout();
}

/* Continue parsing */
Expand All @@ -255,8 +254,8 @@ struct HttpContext {
/* We still have some more data coming in later, so reset timeout */
/* Only reset timeout if we got enough bytes (16kb/sec) since last time we reset here */
httpResponseData->received_bytes_per_timeout += (unsigned int) data.length();
if (httpResponseData->received_bytes_per_timeout >= HTTP_RECEIVE_THROUGHPUT_BYTES * HTTP_IDLE_TIMEOUT_S) {
us_socket_timeout(SSL, (struct us_socket_t *) user, HTTP_IDLE_TIMEOUT_S);
if (httpResponseData->received_bytes_per_timeout >= HTTP_RECEIVE_THROUGHPUT_BYTES * httpResponseData->idleTimeout) {
((HttpResponse<SSL> *) user)->resetTimeout();
httpResponseData->received_bytes_per_timeout = 0;
}
}
Expand Down Expand Up @@ -308,8 +307,7 @@ struct HttpContext {
auto [written, failed] = ((AsyncSocket<SSL> *) returnedSocket)->uncork();
if (failed) {
/* All Http sockets timeout by this, and this behavior match the one in HttpResponse::cork */
/* Warning: both HTTP_IDLE_TIMEOUT_S and HTTP_TIMEOUT_S are 10 seconds and both are used the same */
((AsyncSocket<SSL> *) s)->timeout(HTTP_IDLE_TIMEOUT_S);
((HttpResponse<SSL> *) s)->resetTimeout();
}

/* We need to check if we should close this socket here now */
Expand Down Expand Up @@ -397,7 +395,7 @@ struct HttpContext {
}

/* Expect another writable event, or another request within the timeout */
asyncSocket->timeout(HTTP_IDLE_TIMEOUT_S);
((HttpResponse<SSL> *) s)->resetTimeout();

return s;
});
Expand Down
24 changes: 16 additions & 8 deletions packages/bun-uws/src/HttpResponse.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,27 @@ namespace uWS {
/* Some pre-defined status constants to use with writeStatus */
static const char *HTTP_200_OK = "200 OK";

/* The general timeout for HTTP sockets */
static const int HTTP_TIMEOUT_S = 10;

template <bool SSL>
struct HttpResponse : public AsyncSocket<SSL> {
/* Solely used for getHttpResponseData() */
template <bool> friend struct TemplatedApp;
typedef AsyncSocket<SSL> Super;
public:

HttpResponseData<SSL> *getHttpResponseData() {
return (HttpResponseData<SSL> *) Super::getAsyncSocketData();
}
void setTimeout(uint8_t seconds) {
auto* data = getHttpResponseData();
data->idleTimeout = seconds;
Super::timeout(data->idleTimeout);
}

void resetTimeout() {
auto* data = getHttpResponseData();

Super::timeout(data->idleTimeout);
}
/* Write an unsigned 32-bit integer in hex */
void writeUnsignedHex(unsigned int value) {
char buf[10];
Expand Down Expand Up @@ -140,7 +148,7 @@ struct HttpResponse : public AsyncSocket<SSL> {
}

/* tryEnd can never fail when in chunked mode, since we do not have tryWrite (yet), only write */
Super::timeout(HTTP_TIMEOUT_S);
this->resetTimeout();
return true;
} else {
/* Write content-length on first call */
Expand Down Expand Up @@ -183,7 +191,7 @@ struct HttpResponse : public AsyncSocket<SSL> {

/* If we are now at the end, start a timeout. Also start a timeout if we failed. */
if (!success || httpResponseData->offset == totalSize) {
Super::timeout(HTTP_TIMEOUT_S);
this->resetTimeout();
}

/* Remove onAborted function if we reach the end */
Expand Down Expand Up @@ -358,7 +366,7 @@ struct HttpResponse : public AsyncSocket<SSL> {

HttpResponse *resume() {
Super::resume();
Super::timeout(HTTP_TIMEOUT_S);
this->resetTimeout();
return this;
}

Expand Down Expand Up @@ -475,7 +483,7 @@ struct HttpResponse : public AsyncSocket<SSL> {

auto [written, failed] = Super::write(data.data(), (int) data.length());
if (failed) {
Super::timeout(HTTP_TIMEOUT_S);
this->resetTimeout();
}

/* If we did not fail the write, accept more */
Expand Down Expand Up @@ -534,7 +542,7 @@ struct HttpResponse : public AsyncSocket<SSL> {
if (failed) {
/* For now we only have one single timeout so let's use it */
/* This behavior should equal the behavior in HttpContext when uncorking fails */
Super::timeout(HTTP_TIMEOUT_S);
this->resetTimeout();
}

/* If we have no backbuffer and we are connection close and we responded fully then close */
Expand Down
1 change: 1 addition & 0 deletions packages/bun-uws/src/HttpResponseData.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ struct HttpResponseData : AsyncSocketData<SSL>, HttpParser {
using OnWritableCallback = bool (*)(uWS::HttpResponse<SSL>*, uint64_t, void*);
using OnAbortedCallback = void (*)(uWS::HttpResponse<SSL>*, void*);
using OnDataCallback = void (*)(uWS::HttpResponse<SSL>* response, const char* chunk, size_t chunk_length, bool, void*);
uint8_t idleTimeout = 10; // default HTTP_TIMEOUT 10 seconds

/* When we are done with a response we mark it like so */
void markDone() {
Expand Down
25 changes: 22 additions & 3 deletions src/bun.js/api/server.zig
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ pub const ServerConfig = struct {
} = .{
.tcp = .{},
},

idleTimeout: u8 = 10, //TODO: should we match websocket default idleTimeout of 120?
// TODO: use webkit URL parser instead of bun's
base_url: URL = URL{},
base_uri: string = "",
Expand Down Expand Up @@ -894,6 +894,24 @@ pub const ServerConfig = struct {
return args;
}

if (arg.get(global, "idleTimeout")) |value| {
if (!value.isUndefinedOrNull()) {
if (!value.isAnyInt()) {
JSC.throwInvalidArguments("Bun.serve expects idleTimeout to be an integer", .{}, global, exception);

return args;
}

const idleTimeout: u64 = @intCast(@max(value.toInt64(), 0));
if (idleTimeout > 255) {
JSC.throwInvalidArguments("Bun.serve expects idleTimeout to be 255 or less", .{}, global, exception);
return args;
}

args.idleTimeout = @truncate(idleTimeout);
}
}

if (arg.getTruthy(global, "webSocket") orelse arg.getTruthy(global, "websocket")) |websocket_object| {
if (!websocket_object.isObject()) {
JSC.throwInvalidArguments("Expected websocket to be an object", .{}, global, exception);
Expand Down Expand Up @@ -3914,7 +3932,7 @@ pub const WebSocketServer = struct {
globalObject.throwInvalidArguments("websocket expects idleTimeout to be 960 or less", .{});
return null;
} else if (idleTimeout > 0) {
// uws does not allow idleTimeout to be between (0, 8],
// uws does not allow idleTimeout to be between (0, 8),
// since its timer is not that accurate, therefore round up.
idleTimeout = @max(idleTimeout, 8);
}
Expand Down Expand Up @@ -6306,8 +6324,9 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp
this.vm.eventLoop().debug.exit();
}
}

req.setYield(false);
resp.timeout(this.config.idleTimeout);

var ctx = this.request_pool_allocator.tryGet() catch bun.outOfMemory();
ctx.create(this, req, resp);
this.vm.jsc.reportExtraMemory(@sizeOf(RequestContext));
Expand Down
19 changes: 15 additions & 4 deletions src/deps/libuwsockets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1149,7 +1149,7 @@ extern "C"
data->offset = offset;
data->state |= uWS::HttpResponseData<true>::HTTP_END_CALLED;
data->markDone();
us_socket_timeout(true, (us_socket_t *)uwsRes, uWS::HTTP_TIMEOUT_S);
uwsRes->resetTimeout();
}
else
{
Expand All @@ -1158,9 +1158,20 @@ extern "C"
data->offset = offset;
data->state |= uWS::HttpResponseData<true>::HTTP_END_CALLED;
data->markDone();
us_socket_timeout(0, (us_socket_t *)uwsRes, uWS::HTTP_TIMEOUT_S);
uwsRes->resetTimeout();
}
}

void uws_res_timeout(int ssl, uws_res_r res, uint8_t seconds) {
if (ssl) {
uWS::HttpResponse<true> *uwsRes = (uWS::HttpResponse<true> *)res;
uwsRes->setTimeout(seconds);
} else {
uWS::HttpResponse<false> *uwsRes = (uWS::HttpResponse<false> *)res;
uwsRes->setTimeout(seconds);
}
}

void uws_res_end_without_body(int ssl, uws_res_r res, bool close_connection)
{
if (ssl)
Expand All @@ -1181,7 +1192,7 @@ extern "C"
}
data->state |= uWS::HttpResponseData<true>::HTTP_END_CALLED;
data->markDone();
us_socket_timeout(true, (us_socket_t *)uwsRes, uWS::HTTP_TIMEOUT_S);
uwsRes->resetTimeout();
}
else
{
Expand All @@ -1203,7 +1214,7 @@ extern "C"
}
data->state |= uWS::HttpResponseData<false>::HTTP_END_CALLED;
data->markDone();
us_socket_timeout(false, (us_socket_t *)uwsRes, uWS::HTTP_TIMEOUT_S);
uwsRes->resetTimeout();
}
}

Expand Down
17 changes: 11 additions & 6 deletions src/deps/uws.zig
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ pub const InternalSocket = union(enum) {
detached: void,
pub fn isDetached(this: InternalSocket) bool {
return this == .detached;
}
}
pub fn detach(this: *InternalSocket) void {
this.* = .detached;
}
Expand Down Expand Up @@ -222,7 +222,7 @@ pub fn NewSocketHandler(comptime is_ssl: bool) type {

pub fn ssl(this: ThisSocket) ?*BoringSSL.SSL {
if (comptime is_ssl) {
if(this.getNativeHandle()) |handle| {
if (this.getNativeHandle()) |handle| {
return @as(*BoringSSL.SSL, @ptrCast(handle));
}
return null;
Expand Down Expand Up @@ -674,7 +674,7 @@ pub fn NewSocketHandler(comptime is_ssl: bool) type {
) ?ThisSocket {
const socket_ = ThisSocket{ .socket = .{ .done = us_socket_from_fd(ctx, @sizeOf(*anyopaque), bun.socketcast(handle)) orelse return null } };

if(socket_.ext(*anyopaque)) |holder| {
if (socket_.ext(*anyopaque)) |holder| {
holder.* = this;
}

Expand Down Expand Up @@ -712,7 +712,7 @@ pub fn NewSocketHandler(comptime is_ssl: bool) type {
return error.FailedToOpenSocket;

const socket_ = ThisSocket{ .socket = .{ .done = socket } };
if(socket_.ext(*anyopaque)) |holder| {
if (socket_.ext(*anyopaque)) |holder| {
holder.* = ctx;
}
return socket_;
Expand Down Expand Up @@ -755,7 +755,7 @@ pub fn NewSocketHandler(comptime is_ssl: bool) type {
ThisSocket{
.socket = .{ .connecting = @ptrCast(socket_ptr) },
};
if(socket.ext(*anyopaque)) |holder| {
if (socket.ext(*anyopaque)) |holder| {
holder.* = ptr;
}
return socket;
Expand Down Expand Up @@ -1067,7 +1067,7 @@ pub fn NewSocketHandler(comptime is_ssl: bool) type {
const new_socket = us_socket_context_adopt_socket(comptime ssl_int, socket_ctx, socket, -1) orelse return false;
bun.assert(new_socket == socket);
var adopted = ThisSocket.from(new_socket);
if(adopted.ext(*anyopaque)) |holder| {
if (adopted.ext(*anyopaque)) |holder| {
holder.* = ctx;
}
@field(ctx, socket_field_name) = adopted;
Expand Down Expand Up @@ -2267,6 +2267,9 @@ pub fn NewApp(comptime ssl: bool) type {
pub fn endSendFile(res: *Response, write_offset: u64, close_connection: bool) void {
uws_res_end_sendfile(ssl_flag, res.downcast(), write_offset, close_connection);
}
pub fn timeout(res: *Response, seconds: u8) void {
uws_res_timeout(ssl_flag, res.downcast(), seconds);
}
pub fn write(res: *Response, data: []const u8) bool {
return uws_res_write(ssl_flag, res.downcast(), data.ptr, data.len);
}
Expand Down Expand Up @@ -2679,6 +2682,8 @@ extern fn uws_res_write_header(ssl: i32, res: *uws_res, key: [*c]const u8, key_l
extern fn uws_res_write_header_int(ssl: i32, res: *uws_res, key: [*c]const u8, key_length: usize, value: u64) void;
extern fn uws_res_end_without_body(ssl: i32, res: *uws_res, close_connection: bool) void;
extern fn uws_res_end_sendfile(ssl: i32, res: *uws_res, write_offset: u64, close_connection: bool) void;
extern fn uws_res_timeout(ssl: i32, res: *uws_res, timeout: u8) void;

extern fn uws_res_write(ssl: i32, res: *uws_res, data: [*c]const u8, length: usize) bool;
extern fn uws_res_get_write_offset(ssl: i32, res: *uws_res) u64;
extern fn uws_res_override_write_offset(ssl: i32, res: *uws_res, u64) void;
Expand Down
36 changes: 36 additions & 0 deletions test/js/bun/http/serve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1856,3 +1856,39 @@ it("we should always send date", async () => {
expect(res.headers.has("Date")).toBeTrue();
}
});

it("should allow use of custom timeout", async () => {
using server = Bun.serve({
port: 0,
idleTimeout: 4, // uws precision is in seconds, and lower than 4 seconds is not reliable its timer is not that accurate
async fetch(req) {
const url = new URL(req.url);
return new Response(
new ReadableStream({
async pull(controller) {
controller.enqueue("Hello,");
if (url.pathname === "/timeout") {
await Bun.sleep(5000);
} else {
await Bun.sleep(10);
}
controller.enqueue(" World!");

controller.close();
},
}),
{ headers: { "Content-Type": "text/plain" } },
);
},
});
async function testTimeout(pathname: string, success: boolean) {
const res = await fetch(new URL(pathname, server.url.origin));
expect(res.status).toBe(200);
if (success) {
expect(res.text()).resolves.toBe("Hello, World!");
} else {
expect(res.text()).rejects.toThrow(/The socket connection was closed unexpectedly./);
}
}
await Promise.all([testTimeout("/ok", true), testTimeout("/timeout", false)]);
}, 10_000);

0 comments on commit 384988f

Please sign in to comment.