cloudflare / cloudflare/workerd

🐛 Bug Report — Runtime APIs: unused SQL cursors lead to strange errors

Open
#959 18 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
8.7k
Forks
739
Avg merge
2d 20h
Merged PRs (30d)
174

Description

Hey! 👋 Using the following `workerd` configuration as a template:

```capnp
using Workerd = import "/workerd/workerd.capnp";

const config :Workerd.Config = (
services = [
( name = "main", worker = .worker ),
( name = "storage", disk = ( path = "test-storage", writable = true ) ),
],
sockets = [
( name = "http", address = "*:8080", http = (), service = "main" ),
]
);

const worker :Workerd.Worker = (
compatibilityDate = "2023-08-01",
compatibilityFlags = ["experimental"],
modules = [
( name = "index.mjs", esModule = embed "index.mjs" )
],
durableObjectStorage = ( localDisk = "storage" ),
durableObjectNamespaces = [
( className = "DurableObject", uniqueKey = "DurableObject" )
],
bindings = [
( name = "OBJECT", durableObjectNamespace = "DurableObject" )
],
);
```

```js
// index.mjs
function drain(cursor) {
for (const _ of cursor) {}
}

export class DurableObject {
constructor(state) {
this.state = state;
}

async fetch(request) {
const { pathname } = new URL(request.url);
if (pathname !== "/") return new Response(null, { status: 404 });

const sql = this.state.storage.sql;
sql.exec("DROP TABLE IF EXISTS entries;");
sql.exec("CREATE TABLE entries (key TEXT PRIMARY KEY, value TEXT);");
sql.exec("INSERT INTO entries (key, value) VALUES ('key1', 'value1'), ('key2', 'value2'), ('key3', 'value3');");

{ /* substitute code from below */ }

return new Response("OK");
}
}

export default {
async fetch(request, env, ctx) {
const id = env.OBJECT.idFromName("a");
const stub = env.OBJECT.get(id);
return stub.fetch(request);
}
}
```
---

Substituting...

```js
const selectStmt = sql.prepare("SELECT value FROM entries WHERE key = ?");
const selectCursor = selectStmt("key1");
```

...will give `Error: database table is locked` on the 2nd request to the Durable Object, unless `drain(selectCursor)` is called.

---

Similarly, substituting...

```js
const allCursor = sql.exec("SELECT value FROM entries");
for (const _ of allCursor) break;
```

...to partially read from a cursor, results in the same `Error: database table is locked` on the 2nd request, unless `drain(allCursor)` is also called.

---

Substituting...

```js
const deleteStmt = sql.prepare("DELETE FROM entries WHERE key = ? RETURNING value");
const deleteCursor = deleteStmt("key1");
```

...will give `workerd/util/sqlite.c++:865: failed: SQLite failed; sqlite3_errmsg(db) = cannot commit transaction - SQL statements in progress; sqlite3_step()` on a request to the Durable Object, unless `drain(deleteCursor)` is called, or `RETURNING value` is removed from the statement.

---

I guess it's a bit strange to query data and then not read it all, but it seems like partially reading cursors is something the API permits and encourages. I feel like I'd expect cursors to be dropped at the end of requests, but I guess you could still be using them elsewhere... 😕

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.