jedireza / aqua

:bulb: A website and user system starter
https://jedireza.github.io/aqua/
MIT License
1.38k stars 356 forks source link

actions/api.js ... bug ? #238

Closed walshe closed 7 years ago

walshe commented 7 years ago

I have a rest endpoint locked down with security, and it nicely fails with a 401 code i.e.

server.route({
            method: 'GET',
            path: '/vehicles2',
            config: {
                auth: {
                  strategy: 'session'
                },
                plugins: {
                    'hapi-auth-cookie': {
                        redirectTo: false
                    }
                }
            }

I was hoping that I could nicely handle what do do next in my action, callback

class Actions {
    static getResults(data) {

        console.log('in getResults');
        ApiActions.get(
            '/api/vehicles2',
            data,
            Store,
            Constants.GET_RESULTS,
            Constants.GET_RESULTS_RESPONSE,
            (err, response) => {

                //decide what to do here, if error or status code == 401
            }
        );
    }

but somehow the callback after the store.dispatch() never gets a chance to execute

static makeRequest(request, store, typeReq, typeRes, callback) {

        store.dispatch({
            type: typeReq,
            request
        });

        JsonFetch(request, (err, response) => {

            store.dispatch({
                type: typeRes,
                err,
                response
            });

            if (callback) {
                callback(err, response);
            }
        });
    }

is this a bug is there something I am missing ? I really want to handle rest calls nicely if token is not available for instance

jedireza commented 7 years ago

Your callback should be getting fired. If the response code isn't in the 200s you'll need to check the err value of the callback.

https://github.com/jedireza/aqua/blob/4e7cb75253a04e870b73e61a73842f896ecf983e/client/helpers/json-fetch.js#L38-L62

The best thing you can do is setup some breakpoints in the debugger and watch how the code flows to see what prevents your callback from firing or if something else is wrong.

walshe commented 7 years ago

seems when an err is passed to dispatch that it never comes back to execute the subsequent code, so I had to put in this ugly hack:

    static makeRequest(request, store, typeReq, typeRes, callback) {

        store.dispatch({
            type: typeReq,
            request
        });

        JsonFetch(request, (err, response) => {

            if(callback && err && response.statusCode == 401){
                callback(err, response);
                return
            }

            store.dispatch({
                type: typeRes,
                err,
                response
            });

            if (callback) {
                callback(err, response);
            }
        });
    }
jedireza commented 7 years ago

If an error exists here:

https://github.com/jedireza/aqua/blob/4e7cb75253a04e870b73e61a73842f896ecf983e/client/actions/api.js#L39-L47

nothing is preventing a callback from being fired. Put a breakpoint on line 39 and 45 and confirm that a callback actually exists.

walshe commented 7 years ago

gh

jedireza commented 7 years ago

A 401 would be handled the same as a 400, which we handle today. Like on the login form:

screen shot 2017-08-11 at 12 44 30 pm
jedireza commented 7 years ago

I've shown that stock Aqua does gracefully handle handle 4XX replies from the API.

At this point the best thing to do is create a fork of Aqua with simple changes that reproduce the bug. That way others could clone it and debug themselves.

On the other hand, It sounds like you have started customizing things beyond basic edits, in which case, making a simple reproducible example will probably expose the underlying problem.

This could very well be a bug in Aqua, but we need to pinpoint where it is. Happy hunting. Thanks for being active with the project. 🙏

walshe commented 7 years ago

ok I simplified things a lot and no longer have the issue