Complexity in testing

[sergeykifyak]

Hey guys,
I would like to purpose to you to remove this line, because it adds complexity while testing.

redux-thunk-actions/src/index.js

Line 40 in cb5933d

throw err;

[sbussetti]

@sergeykifyak @machadogj I suppose you could

return Promise.reject(err)

there instead?

It'd have the same result in the promise chain, right?

[machadogj]

Hi @sergeykifyak thanks for bringing this up. Can you describe the complexity you are having? I've been meaning to makes testing of actions easier myself, but I am wondering which complexities this change would fix.

@sbussetti notice that the throw is happening in a .then callback, which makes it return a rejected
promise (so it's essentially the same thing). You can try this with the following code:

const myAction = createActionThunk('BOOM', () => nonExistentFunction());
dispatch(yourAction()).catch(err => console.log('boom', err));

[sbussetti]

Yup, I am assuming @sergeykifyak is directly testing the function outside of a promise, and having trouble catching the thrown exception which is why I suggested the equivalent return of a rejection.

Anywho, just being helpful =)

[machadogj]

@sbussetti well... without a promise, you are right, it's throwing outside of the .then callback. I wonder whether it would make sense to wrap non async calls in promises 🤔

For example, this action will return a promise:

const myAction = createActionThunk('FOO', () => fetch('/orders'));
dispatch(myAction()).then(orders => console.log(orders));

because fetch returns a promise. While this action will return the string directly:

const myAction = createActionThunk('FOO', () => 'bar');
console.log(dispatch(myAction()));

I did it this way so that it could support sync actions without the need of introducing promises, and potentially running things in next ticks.

[sbussetti]

@machadogj yeah really good points. Curious to get more details on how @sergeykifyak is constructing their tests.

[sergeykifyak]

I was to fast when I was opening the issue. My apologies for that.
Let me try to make an example that will make things more clear:
apiActions.js

...
export const loadSomeData = createActionThunk(LOAD_DATA,
    (id, name) => requestData(id, name)
);
...

api.js

const parseJSON = (response) => (
    new Promise((resolve) => response.json()
        .then((json) => resolve({
            response: response,
            serverResponse: json
        })))
);

const request = ({ url, method, headers, body }) => {
    const request = new Request(url, {
            headers: {
                'Content-Type': 'application/x-www-form-urlencoded; charset=UTF-8',
                'SOME-CSRF': 'test',
                'X-Requested-With': 'XMLHttpRequest',
                ...headers
            },
            method,
            body
        });

    return fetch(request)
        .then(parseJSON)
        .then((data) => {
            if (data.response.ok) {
                return data.serverResponse;
            }
            // So I handle all 'unsuccessful' responses as FAILED actions
            throw new XHRError(data);
        });
};

export const requestData = (id, name) => request({
    url: `/api/url/id/name`,
    method: 'GET'
});

I decided to move all unsuccessful requests into '_FAILED' because it is easer to decouple in reducers, like this:
reducerSuccessful.js

case LOAD_DATA1_SUCCEEDED:
case LOAD_DATA2_SUCCEEDED:
case LOAD_DATA3_SUCCEEDED:
            return {
                ...state,
                ...someGeneralReducersHere
            };

reducerUnsuccessful.js

case LOAD_DATA1_FAILED:
case LOAD_DATA2_FAILED:
case LOAD_DATA3_FAILED:
            return {
                ...state,
                ...errorMessageFromServer
            };

Now we can talk about complexity of testing,
I started testing of actions and has issue with testing of '_FAILED' actions. I am using jest for testing, so example:

it('should dispatch LOAD_DATA_FAILED with error message', () => {

        global.fetch = jest.fn().mockImplementation(() => {
            return Promise.resolve({
                ok: false,
                status: 404,
                json: () => Promise.resolve({ message: 'error'})
            })
        });

        const store = mockStore({});

        const expected = expect.arrayContaining([
            { type: ACTION_STARTED, payload: [ 1, 'test' ] },
            { type: ACTION_FAILED, payload: { status: 404, ok: false, ...someMoreInfo } }
        ]);

        return store.dispatch(loadSomeData(1, 'test')).then(() => {
            expect(store.getActions()).toEqual(expected);
        });
    });

1. The problem is that this test will never work, because createActionThunk throws the error right after dispatching actions and jest responses on this like:

Failed: [object Object]

2. It will be nice to have possibility to try catch errors even if they are appear.

Give me know if you will need more info.