1
\$\begingroup\$

This is the first ever bit of front-end code I have ever written, I have tried to adapt the existing call that is there to the backend using axios, so I check on of the response object value using the key like so:

if (result.data.is_cached) { ...

Then I create a function called getStatus, and use a setTimeout to keep calling the function if it is status "pending", but my question is this a "Vue" way of doing things, or is my implementation wrong.

Is this the correct way to handle two post requests?

runSimulation({commit, state}) {
    // Other code above here omitted
    axios.post(process.env.VUE_APP_API_URL + OVERVIEW_URLS.SIMULATE, modifiedInputs, {
        headers: {
            'X-Gitlab-Token': process.env.VUE_APP_X_GITLAB_TOKEN,
        },
    }).then((result) => {
        if (!result.data.is_cached) {
            getStatus(result.data.key);
            function getStatus(taskId) {
                axios.post(process.env.VUE_APP_API_URL + OVERVIEW_URLS.STATUS + taskId, modifiedInputs, {
                    headers: {
                        'X-Gitlab-Token': process.env.VUE_APP_X_GITLAB_TOKEN,
                    },
                }).then((resultStatus) => {

                    if (resultStatus.data.statusFinished === "finished") {
                        commit('setSimulationResults', resultStatus.data.result);
                        commit('toggleGlobalSnackbar', 'resultsReady');
                        commit('updateSimulationStatus', simulationStatuses.COMPLETED);
                    } else if (resultStatus.data.statusFinished === "pending") {
                        // setInterval
                        setTimeout(function() {
                            getStatus(taskId);
                        }, 1000);
                    } else {
                        commit('updateSimulationStatus', simulationStatuses.ERROR);
                    }
                }).catch((e) => {
                    commit('updateSimulationStatus', simulationStatuses.ERROR);
                });

        }  else {
            commit('setSimulationResults', result.data.results);
            commit('toggleGlobalSnackbar', 'resultsReady');
            commit('updateSimulationStatus', simulationStatuses.COMPLETED);
         }

    }).catch((e) => {
        commit('updateSimulationStatus', simulationStatuses.ERROR);
    });
},

EDIT: getStatus function

function getStatus(taskId) {
    axios.post(process.env.VUE_APP_API_URL + OVERVIEW_URLS.STATUS + taskId, modifiedInputs, {
        headers: {
            'X-Gitlab-Token': process.env.VUE_APP_X_GITLAB_TOKEN,
        },
    }).then((resultStatus) => {

        if (resultStatus.data.statusFinished === "finished") {
            commit('setSimulationResults', resultStatus.data.result);
            commit('toggleGlobalSnackbar', 'resultsReady');
            commit('updateSimulationStatus', simulationStatuses.COMPLETED);
        } else if (resultStatus.data.statusFinished === "pending") {
            // setInterval
            setTimeout(function() {
                getStatus(taskId);
            }, 1000);
        } else {
            commit('updateSimulationStatus', simulationStatuses.ERROR);
        }
    }).catch((e) => {
        commit('updateSimulationStatus', simulationStatuses.ERROR);
    });
}
\$\endgroup\$
2
  • \$\begingroup\$ Are you able to show the implementation of getStatus()? Also, I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to edit and give it a different title if there is something more appropriate. \$\endgroup\$ May 10 at 15:56
  • 1
    \$\begingroup\$ @SᴀᴍOnᴇᴌᴀ I have updated the code to show getStatus separately, but it is in my original code as well. I have just put it after the function call, and this is where I don't think this is very vue like, as I have not used it before. \$\endgroup\$
    – mp252
    May 10 at 16:08

1 Answer 1

0
\$\begingroup\$

I can't really tell you if there is a 'Vue' way of doing things... You have to generally handle this by hand. You could do this in the component, but you chose to do it in state which is also fine I think. My comments are going to be more about the code in general than about the Vue way.

You are getting into the promise(d) hell :D the main issue with the readability comes from nested calls. You would be way better off if you used async-await.

async runSimulation({commit, state}) {
    // Other code above here omitted
    try {
        const result = await axios.post(process.env.VUE_APP_API_URL + OVERVIEW_URLS.SIMULATE, modifiedInputs, {
            headers: {
                'X-Gitlab-Token': process.env.VUE_APP_X_GITLAB_TOKEN,
            },
        })

        if (!result.data.is_cached) {
            await getStatus(result.data.key)

            async function getStatus(taskId) {
                try {
                    const resultStatus = await axios.post(process.env.VUE_APP_API_URL + OVERVIEW_URLS.STATUS + taskId, modifiedInputs, {
                        headers: {
                            'X-Gitlab-Token': process.env.VUE_APP_X_GITLAB_TOKEN,
                        },
                    })

                    if (resultStatus.data.statusFinished === 'finished') {
                        commit('setSimulationResults', resultStatus.data.result)
                        commit('toggleGlobalSnackbar', 'resultsReady')
                        commit('updateSimulationStatus', simulationStatuses.COMPLETED)
                    } else if (resultStatus.data.statusFinished === 'pending') {
                        // setInterval
                        setTimeout(function () {
                            getStatus(taskId)
                        }, 1000)
                    } else {
                        commit('updateSimulationStatus', simulationStatuses.ERROR)
                    }
                } catch (e) {
                    commit('updateSimulationStatus', simulationStatuses.ERROR)
                }
            }
        } else {
            commit('setSimulationResults', result.data.results)
            commit('toggleGlobalSnackbar', 'resultsReady')
            commit('updateSimulationStatus', simulationStatuses.COMPLETED)
        }
    } catch (e) {
        commit('updateSimulationStatus', simulationStatuses.ERROR)
    }
}

This is already a bit better, but you can still go further - there is still a lot of nesting going on. In ideal world there would be just one line of indentation which is almost true for the run simulation code, but I will update move the nested function outside.

You are also doing a lot of things inside one method/function. It would be easier to modify state in the root method to what your desired state would be and do the communication with the back end in another method.

async function startSimulation() {
    try {
        return {
            status: simulationStatuses.STARTED,
            result: (await axios.post(process.env.VUE_APP_API_URL + OVERVIEW_URLS.SIMULATE, modifiedInputs, {
                headers: {
                    'X-Gitlab-Token': process.env.VUE_APP_X_GITLAB_TOKEN,
                },
            })).data,
        }
    } catch (e) {
        return {status: simulationStatuses.ERROR, result: null}
    }
}

async function runSimulation({commit, state}) {
    // Other code above here omitted
    const {status, result} = await startSimulation()

    if (result?.is_cached) {
        commit('setSimulationResults', result.results)
        commit('toggleGlobalSnackbar', 'resultsReady')
        commit('updateSimulationStatus', simulationStatuses.COMPLETED)
    } else if (status === simulationStatuses.STARTED) {
        await getStatus(result.data.key)
    } else if (status === simulationStatuses.ERROR) {
        commit('updateSimulationStatus', simulationStatuses.ERROR)
    } else {
        throw new 'But how could you get here?'
    }
}

runSimulation is less cluttered now. The getStatus still remains though.

async function getStatus(taskId) {
    try {
        const { data } = await axios.post(process.env.VUE_APP_API_URL + OVERVIEW_URLS.STATUS + taskId, modifiedInputs, {
            headers: {
                'X-Gitlab-Token': process.env.VUE_APP_X_GITLAB_TOKEN,
            },
        })

        if (data.statusFinished === 'finished') {
            return {
                status: simulationStatuses.COMPLETED,
                resultStatus: data,
            }
        } else if (data.statusFinished === 'pending') {
            return {
                status: simulationStatuses.PENDING,
                resultStatus: data,
            }
        } else {
            return {
                status: simulationStatuses.ERROR,
                resultStatus: null,
            }
        }
    } catch (e) {
        return {
            status: simulationStatuses.ERROR,
            resultStatus: null,
        }
    }
}

async function pollTaskStatusUntilCompleted(commit, taskId) {
    const {status, resultStatus} = await getStatus(taskId)

    if (status === simulationStatuses.COMPLETED) {
        commit('setSimulationResults', resultStatus.result)
        commit('toggleGlobalSnackbar', 'resultsReady')
        commit('updateSimulationStatus', status)
    } else if (status === simulationStatuses.PENDING) {
        setTimeout(function () {
            pollTaskStatusUntilCompleted(taskId)
        }, 1000)
    } else {
        commit('updateSimulationStatus', simulationStatuses.ERROR)
    }
}

This still feels weird with that set timeout in the middle... at least to me. Hopefully the code works and I haven't missed any syntax errors, as I am doing the refactoring kind of blind.

From now on the things I will do are really only cosmetic (even if it is not exactly the way I wanted it to come out)

async function second() {
    return new Promise(resolve => setTimeout(() => resolve(), 1000))
}

// I bet this can be done cleaner, but I am running out of time
async function pollTaskStatusUntilCompleted(taskId) {
    let {status, resultStatus} = await getStatus(taskId)
    while (status === simulationStatuses.PENDING) {
        await second()
        const result = await getStatus(taskId)
        status = result.status
        resultStatus = result.resultStatus
    }

    return {status, resultStatus}
}

async function runSimulation({commit, state}) {
    // Other code above here omitted
    const {status, result} = await startSimulation()

    if (result?.is_cached) {
        commit('setSimulationResults', result.results)
        commit('toggleGlobalSnackbar', 'resultsReady')
        commit('updateSimulationStatus', simulationStatuses.COMPLETED)
    } else if (status === simulationStatuses.STARTED) {
        const {status, resultStatus} = await pollTaskStatusUntilCompleted(result.key)
        if (status === simulationStatuses.COMPLETED) {
            commit('setSimulationResults', resultStatus.result)
            commit('toggleGlobalSnackbar', 'resultsReady')
            commit('updateSimulationStatus', status)
        } else {
            commit('updateSimulationStatus', simulationStatuses.ERROR)
        }
    } else if (status === simulationStatuses.ERROR) {
        commit('updateSimulationStatus', simulationStatuses.ERROR)
    } else {
        throw new 'But how could you get here?'
    }
}

Now all of the commits are at the top level of the runSimulation method. All that is left is just handling of the if statements.

async function runSimulation({commit, state}) {
    function simulationCompleted(results) {
        commit('setSimulationResults', results)
        commit('toggleGlobalSnackbar', 'resultsReady')
        commit('updateSimulationStatus', simulationStatuses.COMPLETED)

        return simulationStatuses.COMPLETED
    }

    function simulationErroredOut() {
        commit('updateSimulationStatus', simulationStatuses.ERROR)
        return simulationStatuses.ERROR
    }

    // Other code above here omitted
    const {status, result} = await startSimulation()

    if (status === simulationStatuses.COMPLETED) {
        return simulationCompleted(result.results)
    } else if (status === simulationStatuses.ERROR) {
        return simulationErroredOut()
    } else {
        const {status: pollStatus, resultStatus} = await pollTaskStatusUntilCompleted(result.key)

        if (pollStatus === simulationStatuses.COMPLETED) {
            return simulationCompleted(resultStatus.results)
        } else {
            return simulationErroredOut()
        }
    }
}

This gives two identical pieces of code or at least almost identical. You can leave it as is or solve it the following way. I am not sure which one I prefer better...

async function awaitSimulationCompletion(taskId, onCompleted, onFailed) {
    const {status, resultStatus} = await pollTaskStatusUntilCompleted(taskId)

    if (status === simulationStatuses.COMPLETED) {
        return onCompleted(resultStatus.results)
    } else {
        return onFailed()
    }
}

async function runSimulation({commit, state}) {
    // Other code above here omitted

    if (status === simulationStatuses.COMPLETED) {
        return simulationCompleted(result.results)
    } else if (status === simulationStatuses.ERROR) {
        return simulationErroredOut()
    } else {
        return await awaitSimulationCompletion(result.key, simulationCompleted, simulationErroredOut)
    }
}

This is the final result of all this refactoring:

async function getStatus(taskId) {
    try {
        const {data} = await axios.post(process.env.VUE_APP_API_URL + OVERVIEW_URLS.STATUS + taskId, modifiedInputs, {
            headers: {
                'X-Gitlab-Token': process.env.VUE_APP_X_GITLAB_TOKEN,
            },
        })


        if (data.statusFinished === 'finished') {
            return {
                status: simulationStatuses.COMPLETED,
                resultStatus: data,
            }
        } else if (data.statusFinished === 'pending') {
            return {
                status: simulationStatuses.PENDING,
                resultStatus: data,
            }
        } else {
            return {
                status: simulationStatuses.ERROR,
                resultStatus: null,
            }
        }
    } catch (e) {
        return {
            status: simulationStatuses.ERROR,
            resultStatus: null,
        }
    }
}

async function second() {
    return new Promise(resolve => setTimeout(() => resolve(), 1000))
}

// I bet this can be done cleaner, but I am running out of time
async function pollTaskStatusUntilCompleted(commit, taskId) {
    let {status, resultStatus} = await getStatus(taskId)
    while (status === simulationStatuses.PENDING) {
        await second()
        const result = await getStatus(taskId)
        status = result.status
        resultStatus = result.resultStatus
    }

    return {status, resultStatus}
}

async function startSimulation() {
    try {
        let result = (await axios.post(process.env.VUE_APP_API_URL + OVERVIEW_URLS.SIMULATE, modifiedInputs, {
            headers: {
                'X-Gitlab-Token': process.env.VUE_APP_X_GITLAB_TOKEN,
            },
        })).data
        return {
            status: result.is_cached ? simulationStatuses.COMPLETED : simulationStatuses.STARTED,
            result,
        }
    } catch (e) {
        return {status: simulationStatuses.ERROR, result: null}
    }
}

async function awaitSimulationCompletion(taskId, onCompleted, onFailed) {
    const {status, resultStatus} = await pollTaskStatusUntilCompleted(taskId)

    if (status === simulationStatuses.COMPLETED) {
        return onCompleted(resultStatus.results)
    } else {
        return onFailed()
    }
}

async function runSimulation({commit, state}) {
    function simulationCompleted(results) {
        commit('setSimulationResults', results)
        commit('toggleGlobalSnackbar', 'resultsReady')
        commit('updateSimulationStatus', simulationStatuses.COMPLETED)

        return simulationStatuses.COMPLETED
    }

    function simulationErroredOut() {
        commit('updateSimulationStatus', simulationStatuses.ERROR)
        return simulationStatuses.ERROR
    }

    // Other code above here omitted
    const {status, result} = await startSimulation()

    if (status === simulationStatuses.COMPLETED) {
        return simulationCompleted(result.results)
    } else if (status === simulationStatuses.ERROR) {
        return simulationErroredOut()
    } else {
        return await awaitSimulationCompletion(result.key, simulationCompleted, simulationErroredOut)
    }
}

One trap is awaiting this call though. With this the behavior is not completely the same to your initial version in that if you await this runSimulation you will be blocked until the simulation is finished. If you do not want this you could separate the run simulation and awaitSimulationCompletion into two calls from your component.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.