ensure handleUnlock is called even for non-cached responses (#70766)

When a response is unsuccessful, we might still have acquired an
incremental cache lock and only realized we couldn't cache it after
receiving a non-success response code. `handleUnlock` is called in the
case of a successful response or a response received during a dynamic
render, but not in the case of an unsuccessful response. This adds the
missing `handleUnlock` for non-cached responses.

This used to be handled appropriately but was lost in some recent
refactors.

No explicit test case was added here as the reproduction in the failing
case was just a hanging build. So if the build passes and does not hang,
the test was successful. Validated it was hanging before implementing
this fix.

packages/next/src/server/lib/patch-fetch.ts

@@ -729,6 +729,10 @@ export function createPatchedFetcher(
               }
             }
 
+            // we had response that we determined shouldn't be cached so we return it
+            // and don't cache it. This also needs to unlock the cache lock we acquired.
+            await handleUnlock()
+
             return res
           })
         }

test/e2e/app-dir/app-fetch-deduping/app-fetch-deduping.test.ts

@@ -1,5 +1,6 @@
 import { findPort, waitFor } from 'next-test-utils'
 import http from 'http'
+import url from 'url'
 import { outdent } from 'outdent'
 import { isNextDev, isNextStart, nextTestSetup } from 'e2e-utils'
 
@@ -9,12 +10,21 @@ describe('app-fetch-deduping', () => {
       const { next } = nextTestSetup({ files: __dirname, skipStart: true })
       let externalServerPort: number
       let externalServer: http.Server
-      let requests = []
+      let successfulRequests = []
 
       beforeAll(async () => {
         externalServerPort = await findPort()
         externalServer = http.createServer((req, res) => {
-          requests.push(req.url)
+          const parsedUrl = url.parse(req.url, true)
+          const overrideStatus = parsedUrl.query.status
+
+          // if the requested url has a "status" search param, override the response status
+          if (overrideStatus) {
+            res.statusCode = Number(overrideStatus)
+          } else {
+            successfulRequests.push(req.url)
+          }
+
           res.end(`Request ${req.url} received at ${Date.now()}`)
         })
 
@@ -30,7 +40,7 @@ describe('app-fetch-deduping', () => {
       })
 
       beforeEach(() => {
-        requests = []
+        successfulRequests = []
       })
 
       afterAll(() => externalServer.close())
@@ -43,7 +53,7 @@ describe('app-fetch-deduping', () => {
           }`
         )
         await next.build()
-        expect(requests.length).toBe(1)
+        expect(successfulRequests.length).toBe(1)
       })
     })
   } else if (isNextDev) {

test/e2e/app-dir/app-fetch-deduping/app/bad-response-page/[slug]/page.js

@@ -0,0 +1,37 @@
+// This page is validating that despite multiple unsuccessful responses to the same, potentially cached URLs,
+// the build locks are resolved and the page is still built successfully.
+export async function generateStaticParams() {
+  return [
+    {
+      slug: 'slug-0',
+    },
+    {
+      slug: 'slug-1',
+    },
+    {
+      slug: 'slug-2',
+    },
+    {
+      slug: 'slug-3',
+    },
+    {
+      slug: 'slug-4',
+    },
+  ]
+}
+
+export default async function Page({ params }) {
+  const data = await fetch(
+    `http://localhost:${process.env.TEST_SERVER_PORT}?status=404`
+  ).then((res) => res.text())
+  await fetch(
+    `http://localhost:${process.env.TEST_SERVER_PORT}?status=404`
+  ).then((res) => res.text())
+
+  return (
+    <>
+      <p>hello world</p>
+      <p id="data">{data}</p>
+    </>
+  )
+}