3

I have some difficulties with a nested promise (below). For now I'm using an async function in the catch to trigger authentication Errors.

But is it really a good way and isn't there a better way ?

The function have to send a POST request. If an authentication Error is thrown then login(), else throw the error. If login() is fulfilled : retry the POST (and then return the results), else throw the error;

function getSomeData() {
    return post('mySpecialMethod');
}

function login() {
    const params = { /* some params */ }
  
    return post('myLoginMethod', params).then(result => {
        /* some processing */
        return { success: true };
    });
}

const loginError = [1, 101];
function post(method, params, isRetry) {
    const options = /* hidden for brevity */;
    
    return request(options)
        // login and retry if authentication Error || throw the error
        .catch(async ex => {
            const err = ex.error.error || ex.error

            if (loginError.includes(err.code) && !isRetry) { // prevent infinite loop if it's impossible to login -> don't retry if already retried
                await login();
                return post(method, params, true)
            } else {
                throw err;
            }
        })
        // return result if no error
        .then(data => {
            // data from 'return request(options)' or 'return post(method, params, true)'
            return data.result
        });
}

Use

getSomeData.then(data => { /* do something with data */});
Community
  • 1
  • 1
Firlfire
  • 413
  • 1
  • 6
  • 14
  • This does not work: you're doing the "*return result if no error*" even if there was an error and you got something back from the retry, on which you are then again accessing `.result`. – Bergi Dec 04 '19 at 14:09

4 Answers4

2

... But is it really a good way and isn't there a better way ?

No it's not a good idea because it hurts code readability.

You're mixing 2 interchangeable concepts, Promise.then/catch and async/await. Mixing them together creates readability overhead in the form of mental context switching.

Anyone reading your code, including you, would need to continuously switch between thinking in terms of asynchronous flows(.then/.catch) vs synchronous flows (async/await).

Use one or the other, preferably the latter since it's more readable, mostly because of it's synchronous flow.

Although I don't agree with how you're handling logins, here's how I would rewrite your code to use async/await and try...catch for exception handling:

function getSomeData() {
    return post('mySpecialMethod')
}

async function login() {
    const params = { } // some params

    await post('myLoginMethod', params)

    return { success: true }
}

const loginError = [1, 101]

async function post(method, params, isRetry) {
    const options = {} // some options

    try {
        const data = await request(options)

        return data.result
    } catch (ex) {
        const err = ex.error.error || ex.error

        if (err) throw err

        if (loginError.includes(err.code) && !isRetry) {
            await login()

            return post(method, params, true)
        }

        throw err
    }
}

I obviously cannot/didn't test the above.

nicholaswmin
  • 21,686
  • 15
  • 91
  • 167
2

I'd suggest that for complex logic at least you use the async/await syntax.

Of course .then() etc is perfectly valid, however you will find the nesting of callbacks awkward to deal with.

My rule (like a lot of things in programming) is use context to make your decision. .then() works nicely when you're dealing with a limited number of promises. This starts to get awkward when you're dealing with more complex logic.

Using async / await for more involved logic allows you to structure your code more like synchronous code, so it's more intuitive and readable.

An example of two approaches is shown below (with the same essential goal). The async / await version is the more readable I believe.

Async / await also makes looping over asynchronous tasks easy, you can use a for loop or a for ... of loop with await and the tasks will be performed in sequence.

function asyncOperation(result) {
    return new Promise(resolve => setTimeout(resolve, 1000, result));
}

async function testAsyncOperationsAwait() {
    const result1 = await asyncOperation("Some result 1");
    console.log("testAsyncOperationsAwait: Result1:", result1);
    const result2 = await asyncOperation("Some result 2");
    console.log("testAsyncOperationsAwait: Result2:", result2);
    const result3 = await asyncOperation("Some result 3");
    console.log("testAsyncOperationsAwait: Result3:", result3);
}


function testAsyncOperationsThen() {
    return asyncOperation("testAsyncOperationsThen: Some result 1").then(result1 => {
        console.log("testAsyncOperationsThen: Result1:", result1);
        return asyncOperation("testAsyncOperationsThen: Some result 2").then(result2 => {
            console.log("testAsyncOperationsThen: Result2:", result2);
            return asyncOperation("testAsyncOperationsThen: Some result 3").then(result3 => {
                console.log("testAsyncOperationsThen: Result3:", result3);
            })
        })
    })
}

    
async function test() {
    await testAsyncOperationsThen();
    await testAsyncOperationsAwait(); 
}

test();
Terry Lennox
  • 29,471
  • 5
  • 28
  • 40
0

Also worth exploring the libraries which provides retry functionalities.

something like https://www.npmjs.com/package/async-retry

Ashish Modi
  • 7,529
  • 2
  • 20
  • 35
-1

Generally this is not a big problem. You can chain/encapsulate async calls like this.

When it comes to logic it depends on your needs. I think the login state of a user should be checked before calling any API methods that require authentication.

Janis Jansen
  • 996
  • 1
  • 16
  • 36