Misleading error when using .nock and .stub

[ScopeyNZ]

Version: 1.4.4

I'm getting a confusing error from the finally definition of .stub:

cannot read property 'pop' of undefined.

This is actually happening because the stub is never initialised, because a call to .nock further up the stack is throwing an error, but this is silently discarded and the stack is interrupted.

You can reproduce this with something like this:

import {test} from '@oclif/test' // I'm using oclif but it's specific to this lib

test
  .nock('https://example.com', int => int.get('https://github.com').reply(200, ''))
  .stub(console, 'log', () => {}) // Doesn't really matter, just need to stub something

This should cause the bug, as the run for .stub will never run, the error caused by the invalid nock configuration should prevent that. The finally for the .stub should still run though, and throw an error.

As far as I can tell, the following line is wrong:

fancy-test/src/base.ts

Line 50 in 79e8c67

if (!await handleError(err)) break

Shouldn't this re-throw the error instead of breaking out of the loop? The only reason I'm hesitant to submit a patch for this is I'm not sure if the parts of the stack that have already been run should be reversed and their .finally callbacks should be run. Happy to submit a patch with a little guidance here.

[ScopeyNZ]

Ok with some further thought/testing I'm pretty sure it's required that the finally calls are still run for the stack that's already completed. I'll submit a patch

[ScopeyNZ]

The plot thickens (a little). I was having trouble reproducing with a test in the fancy-test codebase. It appears my issue is only a problem with newer versions of nock. There's probably other ways to incorrectly configure nock that should still reproduce the problem though.

I don't have time to mess around with a patch any longer but I might be able to pick it up later. Here's the code I had that appeared to be fixing the bug for me:

diff --git a/src/base.ts b/src/base.ts
index ba15294..7cb30fa 100644
--- a/src/base.ts
+++ b/src/base.ts
@@ -47,7 +47,13 @@ const base = <I extends Types.Context>(context: I): Types.Base<I, {}> => {
         try {
           if (next.run) await next.run(context)
         } catch (err) {
-          if (!await handleError(err)) break
+          if (!await handleError(err)) {
+            for (i--; i > 0; i--) {
+              const next = context.chain[i]
+              if (next.finally) next.finally(context)
+            }
+            throw err
+          }
         }
       }
       for (let p of context.chain.reverse()) {