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

Asynchronous method implementations #9

Open
rektide opened this issue Jan 26, 2013 · 27 comments
Open

Asynchronous method implementations #9

rektide opened this issue Jan 26, 2013 · 27 comments

Comments

@rektide
Copy link

rektide commented Jan 26, 2013

Handlers currently must return their results immediately. Feature request: asynchronous execution.

@sidorares
Copy link
Owner

Yes, I was thinking about this, but not sure what API would be best. Always use callbacks (and for simple synchronous handlers just end them with callback(data) instead of return(data)? Or have something like registerHandler and registerAsyncHandler? Can you think of ideal api for registering service for you with example? (provided that you need to tell in some way during registration method name, interface, object path, signature and implementation function reference.

@rektide
Copy link
Author

rektide commented Jan 26, 2013

I have implemented my ideal API. :) It's not tested- I haven't gotten around to testing any of the node-dbus related code I've started- but will be testing it shortly, & will be validating these changes too.

Thanks for the stunningly fast reply and your interest. Hopefully this is somewhat compatible with what you've been thinking and not ugly to you: apologies if it is.

@sidorares
Copy link
Owner

I like your approach but I don't want to depend on any promise/deferred library. I'l try to think of some solution. (otherwise your solution looks good btw)
Note that currently result of handler is always an array (because signature string is a structure even if there is only one type in it and I'm not trying to make 'one type= not a struct' special case) so it's easy to use it as flag: 'non-array' == special value

@sidorares
Copy link
Owner

Hi @rektide ! I think I'm ready to look in this direction again, but I'd use bluebird library instead of Q

@rektide
Copy link
Author

rektide commented Oct 4, 2014

I'll make the changeover this weekend.

The timing here is wonderfuly humorously serendipitous- Thursday night I was back in action writing some experimental Generator based wrappers for node-dbus. Thanks for the fantastic library @sidorares.

@rektide
Copy link
Author

rektide commented Feb 5, 2015

Hey pardon; follow-up here. I thought I had a codebase around that had this at least somewhat implemented, but either that didn't happen or it hasn't turned up! :/ I put in a little effort back in November to try and do the work, but wasn't bamboozling myself a bit/not finding a good-enough approach. Unknown when/if I'll get back to this issue.

@sidorares
Copy link
Owner

no problems :)

@luan007
Copy link

luan007 commented Mar 23, 2015

Hi,
we were also digging into some dbus api and found requirements on async callbacks..
To deal with this w/ minimum impact on dependency structures, a method shell

function(cb) { ... }

can be returned by handler function, then we can achieve async without changing

func.apply

in bus.js,

Would like to see how it goes 🏭

@sidorares
Copy link
Owner

Yeah, I looked at commits in your repo @luan007
We could do the same as what mocha does: if thunk promise or generator is returned from a function, it's assumed that this is an async handler and we respond with data resolved by thunk/promise/generator

@luan007
Copy link

luan007 commented Mar 23, 2015

👍 sounds great, would that be an upcoming feature on mainline repo? 🍊

@sidorares
Copy link
Owner

I'd like to see it in main. Let's see where you are and shape api
Ideally there should be no external dependencies ( thunks ) but I'm ok to depend on bluebird

@luan007
Copy link

luan007 commented Mar 23, 2015

+1 on No external deps,
currently I'm returning a bare

function(cb) { }

in handler function, where cb is in

function cb(err, result) {  }

fasion, err is reflected directly to a dbus-error ( {name:.. message..} )

I wonder if those works for you, if it tastes OK i'll put some more work on error checking and so on
🍊

@sidorares
Copy link
Owner

I'm trying to multitask, can you please outline what we are doing here ( ideally with examples)? :)

My understanding: when I'm exporting a function, currently wi create a mapping "name->implementation function ref" and on receiving invoke message do sync function call ( wrapped in try/catch so that we can send back error ). Proposed change: If function result is a function, treat it as a thunk and only send answer back when callback is called

@sidorares
Copy link
Owner

@luan007 this is mocha discussion on this topic - mochajs/mocha#329

Looks like there is no dependency on external promise library: if it's object with .then property which is a function, assume its a promise.

@luan007
Copy link

luan007 commented Mar 23, 2015

sorry for fuzzy replies, yes you're right that is the point 😺
another proposal is to add dbus-error support, both in sync try-catch and in thunk cb,
it would be great if error caught in try-catch / returned in cb are treated as dbus-errors,
hope all's clear ☕

@sidorares
Copy link
Owner

yes, there should be proper dbus error object sent back when we process async function and it gives error result

@sidorares
Copy link
Owner

@luan007 Also if you can capture any information while you learn this library feel free to put into docs. What we have now in readme sucks :)

@luan007
Copy link

luan007 commented Mar 23, 2015

Or, the node way?

var handlerObj = {
    someMethod: function(arg1, arg2, ..., cb) {
        cb( .. ); 
    }
};

What if everything is async?

@luan007
Copy link

luan007 commented Mar 23, 2015

I see,

var handlerObj = {
      someMethod: function(){
           return { then: function(cb){
                  cb( undefined, result );
           } };
      }
};

in mocha way

@sidorares
Copy link
Owner

well in user code real promise library is likely to be used, it's just we'll duck type test it to avoid dependency

@luan007
Copy link

luan007 commented Mar 23, 2015

unfortunately i'm not into promise lib, need to dig in a bit..
Supporting bare cb and promise at the same time should be the case, will fork again to do real impl ⏩

@tevaum
Copy link

tevaum commented May 12, 2015

Any progress on this stuff, @luan007 ? I'm also needing async calls.

@sidorares
Copy link
Owner

@tevaum @luan007 @rektide could you look at this - 290dbcb

I'm using co to wrap result - it yields immediately for simple types and you can return anything yieldable ( thunk, generator, promise, async/await ) for async flow

Note that co v4.x currently only works if there is global Promise. Not sure what to do with that, I'd like this dbus library to be compatible with node 0.8 - 0.12 for some time. One option is stick with v0.3.x

@luan007
Copy link

luan007 commented Jul 1, 2015

Sorry for the absence, co looks great!

@sidorares
Copy link
Owner

:)

completely forget what state is this at.

If you are playing with async service methods right now - feel free to paste your examples here. These needs to be documented

@TheMSB
Copy link

TheMSB commented Dec 14, 2016

I'm interested to know if anyone has resolved this issue (at least partially) on any branch yet without depending on external libraries? If not I think I will give a shot at implementing it with native promises.

nschoe pushed a commit to nschoe/node-dbus that referenced this issue May 5, 2017
nschoe pushed a commit to nschoe/node-dbus that referenced this issue May 5, 2017
nschoe pushed a commit to nschoe/node-dbus that referenced this issue May 5, 2017
nschoe pushed a commit to nschoe/node-dbus that referenced this issue May 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants