Am I using promisify() with bind() correctly?

Question

I am writing middleware to return database queries using SQLite3 and Next.js. Here is a snippet of code that I wrote to return the contents of a DB table. It appears to work, and my checks of sources appear solid, with mixed reviews about the use of bind().

// route = /dbread
import { NextResponse } from 'next/server';
const sqlite3 = require("sqlite3").verbose();
const util = require('util');

export async function GET(req) {
    const db = new sqlite3.Database("./database.db");
    const dbAll = util.promisify(db.all).bind(db);
    
    let response = await dbAll('SELECT * FROM "text"')
        .then(db.close())
        .catch((err => {
            console.log(err);
            return ['error'];
            }));
    return NextResponse.json(JSON.stringify(response));
}

However, if I take bind out, it breaks, so I keep that in, even though I don't fully understand what it's doing. Is there a better way to do this? The solution seems simple enough, but I just want to make sure I'm stepping on solid ground.

Tangential: creating the database and promisifying all its methods in middleware seems a bit odd, at least to me. I would think you'd want the database reference created on app startup. — Dave Newton, May 26 '23 at 16:08
Hey, that's a good point, and it is tangential too. I've seen the documentation in node mention 'remember to close the database' and I'm really not sure if the scope of closing should be per request or per application instance...I'll do some tests later to see what overhead I am creating by doing this. — chrisroode, May 26 '23 at 17:36

jfriend00 · Accepted Answer · 2023-05-28T23:00:55.513

What you have with .bind(db) works fine and is a correct way of doing things.

I prefer to keep the same type of calling convention that other methods have by doing it like this:

const db = new sqlite3.Database("./database.db");
db.allP = util.promisify(db.all);

And, then you call it like this:

db.allP(...)

like you would call any other method on the database, but that's just my personal preference. Either way will work. Calling this way also "binds" the db instance to the method call so it's properly available as this in the method for the method to use in its implementation.

By way of explanation. When you call a method as in:

obj.method(...)

That makes the obj value available as this in the method's implementation. For your db implementation that is needed.

When you do this:

const dbAll = util.promisify(db.all);
dbAll(...)

dbAll() has become a regular function call and the value of this is either the global object (in non-strict mode) or undefined (if running in strict mode). And, thus the implementation of dbAll() does not have any access to the db variable which would make it not work. It expects db to be in this. Adding .bind() the way you did forces the value of this to be set to db. But, method calls as in obj.method() or db.allP() do this automatically for you. That's why db.allP() doesn't need .bind() to work properly.

FYI, you may want to just use a sql module that has an interface that already supports promises rather than manually promisifying individual methods upon demand.

Thanks a bunch! super helpful. I also like your naming convention, I think I'll adopt that. And yes, I like your suggestion about the module. I think what I will do is write a small batch of module functions that handle all of that, so I can still use the sqlite3 module. That also might get me a solution to optimize the opening / closing of the database. — chrisroode, May 26 '23 at 17:38

Am I using promisify() with bind() correctly?

1 Answers1