Better messages for errors with `Before` and formatter

[plocket]

🤔 What's the problem you're trying to solve?

Though I'm presenting details about a bug in my code, my main purpose in this issue is to request more details in console error output. The details about my bug are to help clarify the situation I'm trying to describe.

Two or three times now I've been frustrated when some seemingly unrelated changes in my code cause Before() to error with no additional information about how to fix the error.

Current situation: I’ve been running my tests successfully. I changed some code and all tests are passing except one. The changes don't affect this particular test at all (I know, I know, but all the other 70+ tests are passing and the Before doesn't have test-related conditional changes). The test isn't failing at a step, it's failing because of the Before(). The output is printing undefined right after the x Before (see below). Here’s what I’m getting from the output:

1) Scenario: My test # test.feature:line
   ✖ Before # steps.js:line
       undefined
   ✔ Given I start the test # steps.js:line
   ✔ And another step # steps.js:line
   ✔ And another step # steps.js:line
   ✔ And another step # steps.js:line
   ✔ After # steps.js:line

1 scenario (1 failed)
4 steps (4 passed)
0m17.605s (executing steps: 0m17.496s)
/Users/repo/node_modules/indent-string/index.js:11
    throw new TypeError(
          ^

TypeError: Expected `input` to be a `string`, got `undefined`
    at module.exports (/Users/repo/node_modules/indent-string/index.js:11:9)
    at formatStep (/Users/repo/node_modules/@cucumber/cucumber/lib/formatter/helpers/test_case_attempt_formatter.js:87:48)
    at /Users/repo/node_modules/@cucumber/cucumber/lib/formatter/helpers/test_case_attempt_formatter.js:101:17
    at Array.forEach (<anonymous>)
    at formatTestCaseAttempt (/Users/repo/node_modules/@cucumber/cucumber/lib/formatter/helpers/test_case_attempt_formatter.js:100:22)
    at formatIssue (/Users/repo/node_modules/@cucumber/cucumber/lib/formatter/helpers/issue_helpers.js:25:94)
    at /Users/repo/node_modules/@cucumber/cucumber/lib/formatter/summary_formatter.js:55:48
    at Array.forEach (<anonymous>)
    at SummaryFormatter.logIssues (/Users/repo/node_modules/@cucumber/cucumber/lib/formatter/summary_formatter.js:54:16)
    at SummaryFormatter.logSummary (/Users/repo/node_modules/@cucumber/cucumber/lib/formatter/summary_formatter.js:41:18)

Node.js v18.17.0

✨ What's your proposed solution?

I'd like to have better clues about what's going wrong. At the moment, I have no idea what to try next.

⛏ Have you considered any alternatives or workarounds?

Not sure this is what you're looking for when you say "alternative solutions", but this is what I've done and what I plan: I've tried to try/catch the Before() with no result. I've tried to move the code in Before() into its own named function so that I might get a line number for whatever's happening, but it hasn't worked. Next I'll try making a new branch off the default and making the changes on it again, going one step at a time.

I've posted in the Slack channel and it'll be nice if someone there can help, but this issue is about getting clearer console output about the problem.

📚 Any additional context?

Not sure it'll stick around forever, or that it's really relevant to this feature request, but here's the new code that's triggering this behavior. This is the Before().

In case it helps, here's the code that the changes were based off of (still passing tests).

---

This text was originally generated from a template, then edited by hand. You can modify the template here.

[plocket]

Having spent a while digging into our bug, I think it's cropping up because we're messing with internal properties of the scenario object in our After() - changing scenario.result.status to FAILED without adding a scenario.result.message. Not sure why that shows up as an error in the Before(). We shouldn't be messing with internal stuff, so I can understand why there isn't a built in message for this. I'll close this issue. That said, it's very confusing that something in After() is causing an x for Before().

Again, appreciate all the work you folks are doing.

[davidjgoss]

From what I can see there, your After hook didn't fail, the error came from the built-in summary formatter trying to summarise the failed scenario afterwards.

In general I would treat anything you're given by Cucumber that you didn't create yourself as immutable. This has actually highlighted that we should really enforce immutability on the parameter passed to these hooks, or at least cloneDeep it each time so mutations are moot. I'd recommend finding another way to influence the status e.g. throw from the After hook.

[plocket]

Thank you for that input. Throwing in After() might work for us.

As far as preventing future similar interference, as a developer I'd prefer inappropriate code manipulation to failed loudly instead of quietly. That is, using only cloneDeep would not tell me, as a developer, why my changes weren't taking effect. My changes would just disappear silently, which I would find confusing. I suppose it could be added to the documentation. Great idea to put explicit bumpers on undesirable behavior, though.