NodeJS: After rejecting promise, the function still is being executed after calling return statement

Question

I have a code which is supposed to check if some repository exists on the remote repository manager and if the result is correct then the execution of the following .then() statements should be stopped.

My main function:

execGitCommand(projectDir, `clone -b ${sourceBranch} ${projectDescription.url} ${projectDir}`, false)
        .then(() => loadProjectPoms(projectDir, projectPoms))
        .then((value) => {
            projectPomDescription = value;
            return checkIfProjectExistsOnNexus(projectPomDescription, projectDescription)
        })
        .then(() => execGitCommand(projectDir, `rebase origin/${destinationBranch}`))
        .then(() => updateProjectVersion(projectDescription, projectDir))
        .then(() => updateProjectPoms(projectDescription, projectDir))
        .then(() => execGitCommand(projectDir,"add -u"))
        .then(() => execGitCommand(projectDir,`commit -m "Set version ${projectDescription.version}"`))
        .then(() => execGitCommand(projectDir,`push origin ${sourceBranch}`)
        .then(waitingCallback)
        .then(() => waitNexusRelease(projectDescription, projectPoms, projectPomDescription, project))
        .then(completeCallback)
        .catch(errorCallback))

So, when the function checkIfProjectExistsOnNexus is rejected i would like to stop executing the next .then commands (.then(() => execGitCommand(projectDir,rebase origin/${destinationBranch})) and so on)

The function checkIfProjectExistsOnNexus:

function checkIfProjectExistsOnNexus(projectPomDescription, projectDescription) {
    return new Promise((resolve, reject) => {
        const version = ((argv.type === 'snapshot') ? projectDescription.version.replace('SNAPSHOT','*') : projectDescription.version);

        nexus.exists(getProjectGroupId(projectPomDescription), projectPomDescription.project.artifactId, version)
                .then((value) => {
                    console.log("value "+ value)
                    if (value) {
                        console.log(`Project: ${projectDescription.name} has already been released in version ${projectDescription.version}, project skipped.`);
                        reject();
                        return;
                    }
                    resolve();
                }).catch(reject);
    }).catch(() => console.log(`Error occurred while checking if project exists on Nexus.`));
}

After calling return after reject() statement the following commands are still being further executed. They should be only executed when function does not go into if statement in checkIfProjectExistsOnNexus function.

Thanks in advance.

you don't need to return` again after a `resolve/reject`. You are returning a promise so the resolved/rejected promise is returned automatically. — Rinkesh P, Oct 15 '22 at 11:04
In your `checkIfProjectExistsOnNexus` return `Promise` there should not be `catch`. That error is supposed to be handled when you call `checkIfProjectExistsOnNexus` — Kaneki21, Oct 15 '22 at 11:08
@RinkeshP you actually need to return after resolve and reject — Konrad, Oct 15 '22 at 11:10
As Kaneki wrote. If you want to keep that `catch()` in `checkIfProjectExistsOnNexus` then make sure to `throw` an error again inside that catch callback, otherwise (as it is now) the promise returned by `catch()` is going to be fulfilled, not rejected. — trincot, Oct 15 '22 at 11:22
Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! — Bergi, Oct 15 '22 at 13:03
I tried it but when I delete the catch() block or re-throw the error I get UnhandledPromiseRejectionWarning. In that case tried to add `catch()` to `checkIfProjectExistsOnNexus` in main functions when that function is called but the error still occurs. — Kamila Nowak, Oct 15 '22 at 18:12
Is there any reason why you don't use `async`/`await`? It would make the whole thing more readable in my opinion. — CherryDT, Oct 15 '22 at 19:51

slebetman · Accepted Answer · 2022-10-15T18:47:31.987

The following .then() are executed because the function is not throwing any errors. The checkIfProjectExistsOnNexus() does not throw any errors because you are catching all errors.

In any programming language (not just javascript but also C++, Java, Ruby etc.) catching an error means stopping the error from propagating to the caller. If you catch an error (either via .catch() in promise chains or via try ... catch blocks) if means you are stopping the error.

To prevent the following .then() from executing make sure your checkIfProjectExistsOnNexus() function throws an error when you want to break execution. You can do that either by deleting the final .catch() inside the function or by rethrowing the error from inside the .catch().

I tried it but when I delete the catch() block or re-throw the error I get UnhandledPromiseRejectionWarning

You are not seeing your mistake because your code is badly formatted. The correct formatting for your code is:

execGitCommand(projectDir, `clone -b ${sourceBranch} ${projectDescription.url} ${projectDir}`, false)
        .then(() => loadProjectPoms(projectDir, projectPoms))
        .then((value) => {
            projectPomDescription = value;
            return checkIfProjectExistsOnNexus(projectPomDescription, projectDescription)
        })
        .then(() => execGitCommand(projectDir, `rebase origin/${destinationBranch}`))
        .then(() => updateProjectVersion(projectDescription, projectDir))
        .then(() => updateProjectPoms(projectDescription, projectDir))
        .then(() => execGitCommand(projectDir,"add -u"))
        .then(() => execGitCommand(projectDir,`commit -m "Set version ${projectDescription.version}"`))
        .then(
            () => execGitCommand(projectDir,`push origin ${sourceBranch}`)
            .then(waitingCallback)
            .then(() => waitNexusRelease(projectDescription, projectPoms, projectPomDescription, project))
            .then(completeCallback)
            .catch(errorCallback)
        );

Once formatted correctly you will notice where you are missing the catch that is causing unhandled promise rejection. You don't have a final catch. That .catch() at the end of the last .then() block is tricking everyone including myself. Even you think it is at the "main" block when it actually is not. It is in the block of the last .then(), not the main block.

Remove it and put it at the end:

execGitCommand(projectDir, `clone -b ${sourceBranch} ${projectDescription.url} ${projectDir}`, false)
        .then(() => loadProjectPoms(projectDir, projectPoms))
        .then((value) => {
            projectPomDescription = value;
            return checkIfProjectExistsOnNexus(projectPomDescription, projectDescription)
        })
        .then(() => execGitCommand(projectDir, `rebase origin/${destinationBranch}`))
        .then(() => updateProjectVersion(projectDescription, projectDir))
        .then(() => updateProjectPoms(projectDescription, projectDir))
        .then(() => execGitCommand(projectDir,"add -u"))
        .then(() => execGitCommand(projectDir,`commit -m "Set version ${projectDescription.version}"`))
        .then(
            () => execGitCommand(projectDir,`push origin ${sourceBranch}`)
            .then(waitingCallback)
            .then(() => waitNexusRelease(projectDescription, projectPoms, projectPomDescription, project))
            .then(completeCallback)
        )
        .catch(errorCallback);

Personally I'd flatten the chain completely:

execGitCommand(projectDir, `clone -b ${sourceBranch} ${projectDescription.url} ${projectDir}`, false)
        .then(() => loadProjectPoms(projectDir, projectPoms))
        .then((value) => {
            projectPomDescription = value;
            return checkIfProjectExistsOnNexus(projectPomDescription, projectDescription)
        })
        .then(() => execGitCommand(projectDir, `rebase origin/${destinationBranch}`))
        .then(() => updateProjectVersion(projectDescription, projectDir))
        .then(() => updateProjectPoms(projectDescription, projectDir))
        .then(() => execGitCommand(projectDir,"add -u"))
        .then(() => execGitCommand(projectDir,`commit -m "Set version ${projectDescription.version}"`))
        .then(() => execGitCommand(projectDir,`push origin ${sourceBranch}`))
        .then(waitingCallback)
        .then(() => waitNexusRelease(projectDescription, projectPoms, projectPomDescription, project))
        .then(completeCallback)
        .catch(errorCallback);

Code formatting is not just something to make your code look nice. It is to help you understand your code and spot bugs. Hopefully this will be a lesson to format your code properly in the future.

I tried it but when I delete the catch() block or re-throw the error I get `UnhandledPromiseRejectionWarning`. — Kamila Nowak, Oct 15 '22 at 18:09
I tried to add to `checkIfProjectExistsOnNexus` the `catch()` block in main function but it didn't helped as well. — Kamila Nowak, Oct 15 '22 at 18:10
Your code is badly formatted. That's why you're not seeing your mistake. — slebetman, Oct 15 '22 at 18:39

score 0 · Answer 2 · answered Oct 15 '22 at 12:49

Replace checkIfProjectExistsOnNexus() function as:

function checkIfProjectExistsOnNexus(projectPomDescription, projectDescription) {
    return new Promise((resolve, reject) => {
        const version = ((argv.type === 'snapshot') ? projectDescription.version.replace('SNAPSHOT', '*') : projectDescription.version);

        nexus.exists(getProjectGroupId(projectPomDescription), projectPomDescription.project.artifactId, version)
            .then((value) => {
                console.log("value " + value)
                if (value) {
                    console.log(`Project: ${projectDescription.name} has already been released in version ${projectDescription.version}, project skipped.`);
                    reject();
                    return;
                }
                resolve();
            }).catch(reject);
    }).catch((error) => {
        console.log(`Error occurred while checking if project exists on Nexus.`);
        throw error;
    });
}

NodeJS: After rejecting promise, the function still is being executed after calling return statement

2 Answers2