Level / level

Universal abstract-level database for Node.js and browsers.
MIT License
1.56k stars 106 forks source link

Bug in promisification of get #97

Closed mitra42 closed 6 years ago

mitra42 commented 6 years ago

It looks to me like get doesn't perform as it should for promises ...

const level = require('level');
async function test() {
        this.config = {dir: "testleveldb."};
        let db = level('testleveldb.Testtable');
        await db.put("testkey", "testval");
        let res = await db.get("testkey");
        console.assert(res === "testval");
}
test();

Should work, i.e. the db.get will return a promise to be awaited. But it errors ...
ReadError: get() requires key and callback arguments

Replacing the 'await db.get` line with a callback e.g.

    db.get("testkey", function(err, res) {
        if (err) { console.log("Err",err); }
        console.log("res=",res)
        console.assert(res === "testval");
    });

Works, so it looks like get is erroneously checking its arguments for a callback when according to the Readme its supposed to allow this to be undefined and return a promise.

vweevers commented 6 years ago

Are you using latest level? Can you please share the stack trace of the ReadError?

mitra42 commented 6 years ago

Thanks for prompt response ... That might be the problem - looks like its using an earlier version of level in a different dependency, let me update and I'll close this issue if it fixes the issue.

mitra42 commented 6 years ago

Confirmed - that fixed it, and the other race condition I was about to report :-)

metaspartan commented 4 years ago

I still have this issue, db.get always wanting a callback even when being used as a promise...Essentially complains that abstract-leveldown doesn't allow get() without a callback...How do use await db.get(); with no callback?

vweevers commented 4 years ago

@carsenk Only the level(up) interface supports promises at the moment, not abstract-leveldown.

metaspartan commented 4 years ago

@carsenk Only the level(up) interface supports promises at the moment, not abstract-leveldown.

Gotcha, know of any plans for abstract-leveldown to support it? As it seems level uses it while using anything Electron?

vweevers commented 4 years ago

There's an ongoing effort to bring the 2 interfaces closer together, but progress is slow for lack of time. Most folks use levelup though (which can wrap any abstract-leveldown implementation); can you explain how/why you're using abstract-leveldown directly?

metaspartan commented 4 years ago

Well I am not using abstract-leveldown directly, it just seems to be used when compiling my app with Electron. I don't have the complaint about db.get() without Electron, when using it, it seems to get caught up in the abstract-leveldown.

vweevers commented 4 years ago

Feel free to open a new issue with details and a repro, so we can take a closer look at what's happening there.