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

SQL: loses connection after timeout #17178

Closed
jakeg opened this issue Feb 8, 2025 · 7 comments · Fixed by #17272
Closed

SQL: loses connection after timeout #17178

jakeg opened this issue Feb 8, 2025 · 7 comments · Fixed by #17272
Assignees
Labels
bug Something isn't working sql Something to do with `sql` in the "bun" module

Comments

@jakeg
Copy link
Contributor

jakeg commented Feb 8, 2025

What version of Bun is running?

1.2.2+c1708ea6a

What platform is your computer?

Linux 5.15.167.4-microsoft-standard-WSL2 x86_64 x86_64

What steps can reproduce the bug?

Sample code:

import { sql } from 'bun'

Bun.serve({
  async fetch (req) {
    await sql`SELECT 'hello world'`
    return new Response('hi')
  }
})

Using a .env file with a POSTGRES_URL that connects to either Supabase's "Session pooler" or their "Transaction pooler".

Works fine until you don't do any requests (and thus SQL queries) for ~30+ seconds, then next time you try a request I presume the pool of connections have been closed due to an idle timeout, but no attempt is made to reconnect. The request hangs then this error appears:

[Bun.serve]: request timed out after 10 seconds. Pass `idleTimeout` to configure.

Am I supposed to do something manually to create new connections in the pool? I assumed that would be automatic.

Only way to fix is to restart the server, then of course it will happen again if no queries for 30+ secondds.

What is the expected behavior?

No response

What do you see instead?

No response

Additional information

No response

@jakeg jakeg added bug Something isn't working needs triage labels Feb 8, 2025
@jakeg
Copy link
Contributor Author

jakeg commented Feb 8, 2025

Here's some code which shows this with just a timeout and no server needed. It seems like it's 60s+ not 30s that causes the issue:

import { sql } from 'bun'

let timeout = 65
let count = 0
let query = async () => {
  console.log(`Trying ${++count}...`)
  await sql`SELECT 'hello world'`
  console.log('worked.')
}

await query()
console.log(`Trying again in ${timeout}s`)
setTimeout(query, timeout * 1_000)

Logs this:

Trying 1...
worked.
Trying again in 65s
Trying 2...

... and never gets further.

@RiskyMH RiskyMH added sql Something to do with `sql` in the "bun" module and removed needs triage labels Feb 8, 2025
@RiskyMH
Copy link
Member

RiskyMH commented Feb 8, 2025

@jakeg can you try canary as I believe this has been fixed.

bun upgrade --canary

@jakeg
Copy link
Contributor Author

jakeg commented Feb 8, 2025

Just upgraded (1.2.3-canary.48+584db03a7) and it's still a bug.

@cirospaciari
Copy link
Member

cirospaciari commented Feb 11, 2025

The error:

[Bun.serve]: request timed out after 10 seconds. Pass `idleTimeout` to configure.

In this caseidleTimeout is from Bun.serve take a look in https://bun.sh/docs/api/http#idletimeout and https://bun.sh/docs/api/http#server-timeout-request-seconds-custom-request-timeouts

SQL connection should reconnect if an idleTimeout happens but its not the error that showed.
For the SQL it self you can increase the maxLifetime of the connection and idleTimeout so the query can be completed. See https://bun.sh/docs/api/sql#connection-options.

const sql = new SQL({
  host: "localhost",
  port: 5432,
  user: "bun",
  password: "bunbunbun",
  database: "bun",
  idleTimeout: 120,
});
const server = Bun.serve({
  idleTimeout: 120, // in seconds, 0 to disable but be careful
  async fetch(req) {
    if (req.url.endsWith("/timeout")) {
      await sql`SELECT pg_sleep(30)`; // will take 30 seconds to complete
    } else {
      await sql`SELECT 1`;
    }
    return new Response("hi");
  },
});

await fetch(server.url + "/timeout")
  .then(console.info)
  .catch(console.error);
await fetch(server.url).then(console.info).catch(console.error);

or you can also use server.timeout only when needed (safer)

const server = Bun.serve({
  async fetch(req) {
    if (req.url.endsWith("/timeout")) {
      server.timeout(req, 120); // long query expected
      await sql`SELECT pg_sleep(30)`; // will take 30 seconds to complete
    } else {
      await sql`SELECT 1`;
    }
    return new Response("hi");
  },
});

using the code I showed but without setting idleTimeout we get the output:

error: The socket connection was closed unexpectedly. For more information, pass `verbose: true` in the second argument to fetch()
  path: "http://localhost:3000//timeout",
 errno: 0,
  code: "ConnectionClosed"


Response (2 bytes) {
  ok: true,
  url: "http://localhost:3000/",
  status: 200,
  statusText: "OK",
  headers: Headers {
    "content-type": "text/plain;charset=utf-8",
    "date": "Tue, 11 Feb 2025 19:39:31 GMT",
    "content-length": "2",
  },
  redirected: false,
  bodyUsed: false,
  Blob (2 bytes)
}

Setting the proper timeout expecting long queries:

Response (2 bytes) {
  ok: true,
  url: "http://localhost:3000//timeout",
  status: 200,
  statusText: "OK",
  headers: Headers {
    "content-type": "text/plain;charset=utf-8",
    "date": "Tue, 11 Feb 2025 19:40:41 GMT",
    "content-length": "2",
  },
  redirected: false,
  bodyUsed: false,
  Blob (2 bytes)
}
Response (2 bytes) {
  ok: true,
  url: "http://localhost:3000/",
  status: 200,
  statusText: "OK",
  headers: Headers {
    "content-type": "text/plain;charset=utf-8",
    "date": "Tue, 11 Feb 2025 19:40:41 GMT",
    "content-length": "2",
  },
  redirected: false,
  bodyUsed: false,
  Blob (2 bytes)
}

In this test even after a connection closed I was able to run another query just fine in latest canary revision 1.2.3-canary.63+321500c62. Will investigate the retries using only Bun.SQL and update here.

Update:

const sql = new SQL({
  host: "localhost",
  port: 5432,
  user: "bun",
  password: "bunbunbun",
  database: "bun",
  // lets guarantee that we only have 1 connection available for this test
  max: 1, 
  // lets make the connection actually close after 30s and wait more 35s after this
  maxLifetime: 30, 
  idleTimeout: 30, 
});

let timeout = 65;
let count = 0;
let query = async () => {
  console.log(`Trying ${++count}...`);
  await sql`SELECT 'hello world'`;
  console.log("worked.");
};

await query();
console.log(`Trying again in ${timeout}s`);
setTimeout(query, timeout * 1_000);
Trying 1...
worked.
Trying again in 65s
Trying 2...
worked.

@jakeg can you check if you can still reproduce this bug?

@cirospaciari cirospaciari reopened this Feb 11, 2025
@cirospaciari cirospaciari self-assigned this Feb 11, 2025
@jakeg
Copy link
Contributor Author

jakeg commented Feb 11, 2025

@cirospaciari sorry, my original bug report should have never included the Bun.serve() code - I understand the idleTimeout is from there and that just caused confusion.

I'm using Supabase PostgreSQL here with their default settings. I'm still getting the same bug (trying on canary 1.2.3-canary.63+321500c62). Here's the code I'm using:

import { sql } from 'bun'

let timeout = 65
let count = 0
let query = async () => {
  console.log(`Trying ${++count}...`)
  await sql`SELECT 'hello world'`
  console.log('worked.')
}

await query()
console.log(`Trying again in ${timeout}s`)
setTimeout(query, timeout * 1_000)

And the output:

Trying 1...
worked.
Trying again in 65s
Trying 2...

... at which point it just hangs still, seemingly forever.

I don't have a local PostgreSQL database I can try against, just the Supabase one. Maybe its based on some sort of Supabase default? But again, I would expect Bun to be reconnecting or something, not just hanging.

I just tried changing to this instead:

import { SQL } from 'bun'

let sql = new SQL({
  url: process.env.POSTGRES_URL,
  maxLifetime: 0
})

// ...

... and still hanging.

This, however, works without hanging:

let sql = new SQL({
  url: process.env.POSTGRES_URL,
  idleTimeout: 30
})

Is this expected behaviour? I'm guessing not.

@cirospaciari
Copy link
Member

cirospaciari commented Feb 11, 2025

@cirospaciari sorry, my original bug report should have never included the Bun.serve() code - I understand the idleTimeout is from there and that just caused confusion.

I'm using Supabase PostgreSQL here with their default settings. I'm still getting the same bug (trying on canary 1.2.3-canary.63+321500c62). Here's the code I'm using:

import { sql } from 'bun'

let timeout = 65
let count = 0
let query = async () => {
console.log(Trying ${++count}...)
await sqlSELECT 'hello world'
console.log('worked.')
}

await query()
console.log(Trying again in ${timeout}s)
setTimeout(query, timeout * 1_000)
And the output:

Trying 1...
worked.
Trying again in 65s
Trying 2...

... at which point it just hangs still, seemingly forever.

I don't have a local PostgreSQL database I can try against, just the Supabase one. Maybe its based on some sort of Supabase default? But again, I would expect Bun to be reconnecting or something, not just hanging.

I just tried changing to this instead:

import { SQL } from 'bun'

let sql = new SQL({
url: process.env.POSTGRES_URL,
maxLifetime: 0
})

// ...
... and still hanging.

This, however, works without hanging:

let sql = new SQL({
url: process.env.POSTGRES_URL,
idleTimeout: 30
})
Is this expected behaviour? I'm guessing not.

Is not the expected behavior, really weird that you are facing this, will check using supabase, I cannot replicate this error with a local database with is weird, thank you for all information will investigate further.

Some notes:

By default idleTimeout is 0 and maxLifetime is 0 but connectionTimeout is 30s

My guess without setting maxLifeTime or idleTimeout the server connection with supabase just got unusable and we dont trigger the close properly and just hang, for now setting both maxLifetime and idleTimeout would be the recommended approach, will open a PR to match the same max_lifetime behavior as postgres.js

@jakeg can you say if you are using Session pooler, Transaction pooler or Direct connection from supabase?

@jakeg
Copy link
Contributor Author

jakeg commented Feb 12, 2025

Can confirm that on Canary (1.2.3-canary.70+1bb1f59b9) since the PR landed this now works with just import { sql } from 'bun'. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sql Something to do with `sql` in the "bun" module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants