Improve error logging

Author: mike-plummer

packages/proxy/lib/http/index.ts

@@ -22,6 +22,7 @@ import { DeferredSourceMapCache } from '@packages/rewriter'
 import type { RemoteStates } from '@packages/server/lib/remote_states'
 import type { CookieJar } from '@packages/server/lib/cookie-jar'
 import type { Automation } from '@packages/server/lib/automation/automation'
+import { errorUtils } from '@packages/errors'
 
 function getRandomColorFn () {
   return chalk.hex(`#${Number(
@@ -156,11 +157,13 @@ export function _runStage (type: HttpStages, ctx: any, onError) {
       const fullCtx = {
         next: () => {
           fullCtx.next = () => {
-            if (type === HttpStages.Error) {
-              ctx.debug('`this.next` called more than once within the same middleware function but was ignored due to being in an Error flow. %o', { middlewareName, error: ctx.error })
-            } else {
-              throw new Error(`Error running proxy middleware: Cannot call this.next() more than once in the same middleware function. Doing so can cause unintended issues.`)
+            const error = new Error('Error running proxy middleware: Detected `this.next()` was called more than once in the same middleware function. This can cause unintended issues and may indicate an error in your middleware logic or configuration.')
+
+            if (ctx.error) {
+              error.message = error.message += `\nThis middleware invocation previously encountered an error which may be related: ${ctx.error}`
             }
+
+            throw error
           }
 
           copyChangedCtx()
@@ -194,10 +197,7 @@ export function _runStage (type: HttpStages, ctx: any, onError) {
       try {
         middleware.call(fullCtx)
       } catch (err) {
-        // TODO Find better way to log this
-        // eslint-disable-next-line no-console
-        console.error(err)
-
+        errorUtils.logError(err)
         // This is a total failure so we will not be retrying the request, clear it before executing handler
         // If we do not do this we will end up attempting to double-execute the middleware `next` method
         // https://github.com/cypress-io/cypress/issues/22825

packages/proxy/test/unit/http/response-middleware.spec.ts

@@ -31,33 +31,55 @@ describe('http/response-middleware', function () {
     ])
   })
 
-  it('errors if this.next() is called more than once in the same middleware', function (done) {
-    const middleware = function () {
-      this.next()
-      this.next()
-    }
+  describe('multiple this.next invocations', () => {
+    context('within the same middleware', () => {
+      it('throws an error', function (done) {
+        const middleware = function () {
+          this.next()
+          this.next()
+        }
+
+        testMiddleware([middleware], {
+          onError (err) {
+            expect(err.message).to.equal('Error running proxy middleware: Detected `this.next()` was called more than once in the same middleware function. This can cause unintended issues and may indicate an error in your middleware logic or configuration.')
+
+            done()
+          },
+        })
+      })
 
-    testMiddleware([middleware], {
-      onError (err) {
-        expect(err.message).to.equal('Error running proxy middleware: Cannot call this.next() more than once in the same middleware function. Doing so can cause unintended issues.')
+      it('includes a previous context error in error message if one exists', (done) => {
+        const middleware = function () {
+          this.next()
+          this.next()
+        }
 
-        done()
-      },
+        testMiddleware([middleware], {
+          error: new Error('previous error'),
+          onError (err) {
+            expect(err.message).to.contain('This middleware invocation previously encountered an error which may be related: Error: previous error')
+
+            done()
+          },
+        })
+      })
     })
-  })
 
-  it('does not error if this.next() is called more than once in different middleware', function () {
-    const middleware1 = function () {
-      this.next()
-    }
-    const middleware2 = function () {
-      this.next()
-    }
+    context('across different middleware', () => {
+      it('does not throw an error', function () {
+        const middleware1 = function () {
+          this.next()
+        }
+        const middleware2 = function () {
+          this.next()
+        }
 
-    return testMiddleware([middleware1, middleware2], {
-      onError () {
-        throw new Error('onError should not be called')
-      },
+        return testMiddleware([middleware1, middleware2], {
+          onError () {
+            throw new Error('onError should not be called')
+          },
+        })
+      })
     })
   })