From 7e9f9bfbef2cf4d63ceb899eeccf5bd0fcba2ec5 Mon Sep 17 00:00:00 2001
From: Steven <steven@ceriously.com>
Date: Mon, 22 Aug 2022 17:37:16 -0400
Subject: [PATCH] Fix  `onError` handling in `next/future/image` (#39824)

This PR fixes a race condition where the `onError` handler was never called because the error happened before react attached the error handler.
---
 docs/api-reference/next/future/image.md       |  5 +----
 packages/next/client/future/image.tsx         | 17 ++++++++++++--
 .../pages/on-error-before-hydration.js        | 22 +++++++++++++++++++
 .../image-future/default/test/index.test.ts   |  8 +++++++
 4 files changed, 46 insertions(+), 6 deletions(-)
 create mode 100644 test/integration/image-future/default/pages/on-error-before-hydration.js

diff --git a/docs/api-reference/next/future/image.md b/docs/api-reference/next/future/image.md
index 2438cad0c84e..aeeacf7eac26 100644
--- a/docs/api-reference/next/future/image.md
+++ b/docs/api-reference/next/future/image.md
@@ -43,7 +43,6 @@ Compared to `next/image`, the new `next/future/image` component has the followin
   - Removes `lazyBoundary` prop since there is no native equivalent
   - Removes `lazyRoot` prop since there is no native equivalent
 - Removes `loader` config in favor of [`loader`](#loader) prop
-- Note: the [`onError`](#onerror) prop might behave differently
 
 ## Known Browser Bugs
 
@@ -316,7 +315,7 @@ The callback function will be called with one argument, an object with the follo
 
 A callback function that is invoked when the image is loaded.
 
-Note that the load event might occur before client-side hydration completes, so this callback might not be invoked in that case.
+Note that the load event might occur before the placeholder is removed and the image is fully decoded.
 
 Instead, use [`onLoadingComplete`](#onloadingcomplete).
 
@@ -324,8 +323,6 @@ Instead, use [`onLoadingComplete`](#onloadingcomplete).
 
 A callback function that is invoked if the image fails to load.
 
-Note that the error might occur before client-side hydration completes, so this callback might not be invoked in that case.
-
 ### loading
 
 > **Attention**: This property is only meant for advanced usage. Switching an
diff --git a/packages/next/client/future/image.tsx b/packages/next/client/future/image.tsx
index 81cc5e45d964..5fe483f337fe 100644
--- a/packages/next/client/future/image.tsx
+++ b/packages/next/client/future/image.tsx
@@ -358,7 +358,14 @@ const ImageElement = ({
         loading={loading}
         style={{ ...imgStyle, ...blurStyle }}
         ref={useCallback(
-          (img: ImgElementWithDataProp) => {
+          (img: ImgElementWithDataProp | null) => {
+            if (img && onError) {
+              // If the image has an error before react hydrates, then the error is lost.
+              // The workaround is to wait until the image is mounted which is after hydration,
+              // then we set the src again to trigger the error handler (if there was an error).
+              // eslint-disable-next-line no-self-assign
+              img.src = img.src
+            }
             if (process.env.NODE_ENV !== 'production') {
               if (img && !srcString) {
                 console.error(`Image is missing required "src" property:`, img)
@@ -374,7 +381,13 @@ const ImageElement = ({
               )
             }
           },
-          [srcString, placeholder, onLoadingCompleteRef, setBlurComplete]
+          [
+            srcString,
+            placeholder,
+            onLoadingCompleteRef,
+            setBlurComplete,
+            onError,
+          ]
         )}
         onLoad={(event) => {
           const img = event.currentTarget as ImgElementWithDataProp
diff --git a/test/integration/image-future/default/pages/on-error-before-hydration.js b/test/integration/image-future/default/pages/on-error-before-hydration.js
new file mode 100644
index 000000000000..ac68b482a3fc
--- /dev/null
+++ b/test/integration/image-future/default/pages/on-error-before-hydration.js
@@ -0,0 +1,22 @@
+import { useState } from 'react'
+import Image from 'next/future/image'
+
+const Page = () => {
+  const [msg, setMsg] = useState(`default state`)
+  return (
+    <div>
+      <Image
+        id="img"
+        src="/404.jpg"
+        width="200"
+        height="200"
+        onError={() => {
+          setMsg(`error state`)
+        }}
+      />
+      <p id="msg">{msg}</p>
+    </div>
+  )
+}
+
+export default Page
diff --git a/test/integration/image-future/default/test/index.test.ts b/test/integration/image-future/default/test/index.test.ts
index bf6a00d046e4..a21d1112da25 100644
--- a/test/integration/image-future/default/test/index.test.ts
+++ b/test/integration/image-future/default/test/index.test.ts
@@ -387,6 +387,14 @@ function runTests(mode) {
     )
   })
 
+  it('should callback native onError even when error before hydration', async () => {
+    let browser = await webdriver(appPort, '/on-error-before-hydration')
+    await check(
+      () => browser.eval(`document.getElementById("msg").textContent`),
+      'error state'
+    )
+  })
+
   it('should work with image with blob src', async () => {
     let browser
     try {