thingdom/node-neo4j

Allow for promises in addition to callbacks

jrgleason opened this issue ยท 15 comments

This would be awesome and then I could stop wrapping everything :-)

๐Ÿ‘ x 100
I'm currently using bluebird.promisify, but there must be a better way ๐Ÿ˜‰

@bojidar-bg Me too

var db = new neo4j.GraphDatabase(conf.connectionString),
Promise = require('bluebird'),
Cypher;

if(db.cypher)
   Cypher = Promise.promisify(db.cypher.bind(db));
...
return Cypher({
    query: query,
    params: params
})

Feedback noted! =) Let me wrap up v2 and ship it as-is, and then I'll look to add promises soon after.

Not to spoil the party: I could definitely applaud node-neo4j going all out for promises, but aren't there more important things to do when you could just use bluebird.promisifyAll which does the job just as well?

What I mean is that right now you can just do the following once somewhere in your node application (ES2015 example)

import Promise from 'bluebird'
import neo4j from 'neo4j'
Promise.promisifyAll(neo4j)

et voila, instant node-neo4j with promises all over the place. No wrapping involved; you just call the same methods you normally do with Async attached to the function name (see also https://github.com/petkaantonov/bluebird/blob/master/API.md#promisepromisifyallobject-target--object-options---object)

Of course, the above assumes you are using bluebird which I can understand not everyone is doing.

@hilkeheremans ~ I don't really like to type Async or Promise after every function call I make...

@bojidar-bg Maybe I'm missing something, please help me understand the complete issue for you?

My take on it: after promisifying it all, you can easily wrap neo4j's Async calls in a wrapper library with the same function names (=nearly zero effort). The api isn't that large so that shouldn't be a big issue?

For the async remark an example (without proper error handling but you get my drift)

    let query = function(params) {
        "use strict";
        return Neo4j.db.cypherAsync(params)
    }

Not sure where you are going with the "typing Promise" after every function call you make. Coul you explain what you mean?

I'm personally combining all this with ES7 async/await (read: Babel transpiling), where you can, using the suggestion I made above, essentially end up with the following, IMO very practical code (another quick 'n dirty example):

function async doSomeQuery() {
    try {
        let result = await neo4jwrapper.query({
                query: 'some_query',
                params: {
                    prop1: 'value'
                }
            })
    } catch(err) {
        console.error(err)
    }
}

How would this make things harder (*)?

A native implementation is better and I would certainly support it, but I personally prefer @aseemk focuses on other issues since this is an easy solution with minimal time effort, which also doesn't imply having to maintain two API surfaces (callbacks & promises) in the future. But don't let that stop anyone from doing this anyway. :-)

(*) aside from error handling which is a little easier to forget with Promises & await/async.

I also seem to have issues if I do multiple promises (using bluebird) within the same transaction. I get the following error...

Unhandled rejection neo4j.ClientError: A request within this transaction is curr... (length: 1465)

Why does it matter if one is already running? Couldn't it just queue the Cypher query?

That limitation comes from Neo4j itself: concurrent queries within the same transaction are not allowed.

You have an interesting and good point that node-neo4j could queue the queries under the hood. I hesitate with something like that though, as part of node-neo4j's goals is to be transparent. It feels like it's worth knowing that the queries aren't running concurrently, rather than node-neo4j hiding that.

I survived this by reverting back from Promises and using the async library. I also opened this #172 because I think it would be nice for it to be done on the node-neo4j side so I can still use promises :-)

@jrgleason Have you tried bluebird's Promise.map with {concurrency:1} as an option?

Promise.map([array_of_values],function(value) { // return a promise }, {concurrency:1})

Promise.map allows you to execute a variable number of promises, based on either a function array or value array, and the concurrency option ensures that bluebird will only execute one promise at a time, only starting the next once the previous one has resolved.

I use bluebirds promisifyAll to get pomified neo4j methods. Could be minimal lines to get it in.

Can someone send me the code snippet how to promisifyAll neo4j

var Promise = require('bluebird');
var neo4j = require('neo4j');
Promise.promisifyAll(neo4j);

This doesn't work for me.

hi guys. Any update on promises with thingdom node-neo4j?
Thanks!