[Enhancement] Refactor handler(Raw|Http) calls to use Promises

[pocesar]

To the implementators and extensions POV, refactoring a standard way using promises in those methods increase testability, interop and easier integrations. It also decouples the functionality that is currently ingrained inside those methods, and expose them for better unit testing

[colceagus]

We could split this part into two, if you need help, to get some speed for reaching the milestone faster and focus on more improvements/refactoring/cleaning the code/testing.

What do you think, @pocesar ?

[pocesar]

I'll push some of the things I've done so far, what is nagging me is the parser inside the handler methods

[colceagus]

In what way ? what's the bugging part?

[colceagus]

i synced the fork with this upstream, but I see no changes or commits.

[pocesar]

I didn't push yet, it's a mess (only 4 tests passing), but if you want it anyway, I can push as-is

[pocesar]

ok I put it in the develop branch, it's really broken right now. some places are creating internal classes inside, and expect them to handleMessage. I can either pass the conn to the promise result (would be better for control, and for integration) or I can just handle everything the way it is right now, and just resolve the promise when it's done (going to bed now)

[colceagus]

I think I have some free time this weekend, after this I'm going to start working again. If you think you need a hand, just say, and I'll jump in.

Cheers! Onto the v2.0 of json-rpc2!

[pocesar]

alright, if you can, I won't be able to work on it from today through sunday though. one thing, I removed the Promise.method from inside the checkAuth because it's an expensive call (it uses new Function under the hood), so having it in a frequently called function could have performance implications

[colceagus]

What if we use it only once, when enabling the authorization handler type
(enableBasic/Cookie/JWTAuth) instead of every time we check the
authorization headers? I think this might solve the performance issue and
still be able to support synchronous implementations of the handler?

On Sat, Oct 8, 2016 at 1:47 AM, Paulo Cesar notifications@github.com
wrote:

alright, if you can, I won't be able to work on it from today through
sunday though. one thing, I removed the Promise.method from inside the
checkAuth because it's an expensive call (it uses new Function under the
hood), so having it in a frequently called function could have performance
implications

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#29 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABzKA2wdJmnvPw8iTTFmpaMW-dnNkLERks5qxsvmgaJpZM4KOByg
.

[colceagus]

Would you mind if I try a clean implementation (from where I left off) and review it in a new pull request?

If not, I will try to continue from the last commit.