cucumber / cucumber-cpp

Support for writing Cucumber step definitions in C++
MIT License
306 stars 131 forks source link

Uncaught exception in client code does not release contextReference object in ScenarioScope #226

Open RajmundKail opened 4 years ago

RajmundKail commented 4 years ago

Summary

When the test client code has a function call inside After step and this function call throws an exception, then cucumber-cpp does not catch this error on server side thus yields undefined behavior. This uncaught exception causes endScenario() function abort and won't release the current scenario scope from memory. This impacts all the scenarios after this and causes undefined method '[]' for nil:NilClass (NoMethodError) error in the wire server. It seems the uncaught exception goes straight into the parser and shadows the real problem.

Expected Behavior

Every each test scenario should be isolated from each other scenario and the hooks should also handle any possible exception to ensure the test scenario remains in stable state. Cucumber should manage the lifetime of the scenario object properly and it should release its resources even if an exception is thrown in the client side.

Current Behavior

The wire server does not support responses on hooks at the moment. If there is an uncaught exception in the after step for example, then the uncaught exception impacts the current test scenario. The endScenario() function stops abruptly without releasing its resources.

The below error message is shown in the console when there is an exception in after step:

undefined method `[]' for nil:NilClass (NoMethodError)

Possible Solution

The endScenario should release the resources of the current scenario even if an exception occurs. A try... catch block should do the trick in this code block like this:

void CukeCommands::endScenario() {
    std::exception_ptr eptr;
    try {
        HookRegistrar::execAfterHooks(currentScenario.get());
    } catch (...) {
        eptr = std::current_exception();
    }
    contextManager.purgeContexts();
    currentScenario.reset();
    if (eptr != nullptr) {
        std::rethrow_exception(eptr);
    }
}

This fix on its own would not fix the problem altogether however... The Hook object should also implement methods that communicates with the wire server and reports any possible errors that might occur. The Hook class should have similar behavior as BasicStep and it should return the InvokeResult accordingly.

InvokeResult BasicStep::invoke(const InvokeArgs *pArgs) {
    this->pArgs = pArgs;
    currentArgIndex = 0;
    currentResult = InvokeResult::success();
    try {
        InvokeResult returnedResult = invokeStepBody();
        if (currentResult.isPending()) {
            return currentResult;
        } else {
            return returnedResult;
        }
    } catch (const std::exception& ex) {
        return InvokeResult::failure(ex.what());
    } catch (const std::string& ex) {
        return InvokeResult::failure(ex);
    } catch (const char *ex) {
        return InvokeResult::failure(ex);
    } catch (...) {
        // Cucumber needs a description here
        return InvokeResult::failure("Unknown exception");
    }
}

Steps to Reproduce (for bugs)

  1. In your test code, create an AFTER() step.
  2. Place a simple throw -1 inside it.
  3. See what happens.

Context & Motivation

We are using cucumber-cpp to test the behavior of our product.

Your Environment

RajmundKail commented 4 years ago

Question: Is it intentional that only BasicStep is communicating with the wire server? If any exception occurs in Hook (Before, After), then the exception is not caught by cucumber and the exception goes straight to the parser that throws the infamous undefined method '[]' for nil:NilClass (NoMethodError) error.

ursfassler commented 8 months ago

I can reproduce the problem with the current version. Seems to happen for the BEFORE and AFTER hooks.