search

percolatestudio / meteor-google-api

A simple API encapsulating some common patterns regarding Google's APIs
https://atmospherejs.com/percolate/google-api
MIT License
48 stars 30 forks source link

Promises resolve incorrectly on the server #1

Closed tmeasday closed 9 years ago

tmeasday commented 10 years ago

@zol - there's an issue with this library.

If on the server, you make a call that fails due to access token issues, which then uses the refresh token to get a new access token, on the server, it returns the wrong thing. It actually ends up working (i.e. the API call gets made successfully), but the function returns undefined.

I added a test case on the devel branch that highlights it. It's some weird interaction between promises and futures and Meteor.bindEnvironment. I've spent some time working at it but I'm pretty bemused.

zol commented 10 years ago

Ah crap. Maybe it's worth trying the 1.0 release train of Q? Are we pushing shit up hill by trying to create a unified client/server library for this?

tmeasday commented 10 years ago

Possibly to both.

Ideally what we want is a library with a sync API on the server and a promise API on the client that uses the same codebase behind the scenes.

Right now, we’ve implemented that via writing the library with promises, then “converting" the promises to async — thus running into these problems with the interaction of fibers/futures and promises.

I’m wondering if we should just write the lib in a standard async style, then wrap it in promises client side and sync server side. It’ll make the library a bit more difficult to write, but potentially avoid a lot of these difficulties.

On Friday, 20 June 2014 at 4:05 am, Zoltan Olah wrote:

Ah crap. Maybe it's worth trying the 1.0 release train of Q? Are we pushing shit up hill by trying to create a unified client/server library for this?

— Reply to this email directly or view it on GitHub (https://github.com/percolatestudio/meteor-google-api/issues/1#issuecomment-46595544).

zol commented 10 years ago

I think that’s exactly the approach we should take.