ReactImage: propagate error messages to JS (#10492)

microsoft · Jan 4, 2023 · 874960e · 874960e
1 parent a6fa390
commit 874960e
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 28 deletions.
diff --git a/change/react-native-windows-68b9b820-27ca-4ff1-b24f-dd09ca290429.json b/change/react-native-windows-68b9b820-27ca-4ff1-b24f-dd09ca290429.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "ReactImage: propagate error messages to JS",
+  "packageName": "react-native-windows",
+  "email": "ilit@fb.com",
+  "dependentChangeType": "patch"
+}
diff --git a/vnext/Microsoft.ReactNative/Views/Image/ImageViewManager.cpp b/vnext/Microsoft.ReactNative/Views/Image/ImageViewManager.cpp
@@ -89,10 +89,11 @@ class ImageShadowNode : public ShadowNodeBase {
         *reactImage, winrt::Microsoft::ReactNative::AccessibilityRoles::Image);
 
     m_onLoadEndToken = reactImage->OnLoadEnd([imageViewManager{static_cast<ImageViewManager *>(GetViewManager())},
-                                              reactImage](const auto &, const bool &succeeded) {
+                                              reactImage](const auto &, const winrt::hstring &error) {
       ReactImageSource source{reactImage->Source()};
 
-      imageViewManager->EmitImageEvent(reactImage.as<winrt::Grid>(), succeeded ? "topLoad" : "topError", source);
+      imageViewManager->EmitImageEvent(
+          reactImage.as<winrt::Grid>(), error.empty() ? "topLoad" : "topError", source, error);
       imageViewManager->EmitImageEvent(reactImage.as<winrt::Grid>(), "topLoadEnd", source);
     });
   }
@@ -166,12 +167,19 @@ bool ImageViewManager::UpdateProperty(
   return ret;
 }
 
-void ImageViewManager::EmitImageEvent(winrt::Grid grid, const char *eventName, ReactImageSource &source) {
+void ImageViewManager::EmitImageEvent(
+    winrt::Grid grid,
+    const char *eventName,
+    ReactImageSource &source,
+    const winrt::hstring &error) {
   int64_t tag = GetTag(grid);
   folly::dynamic imageSource =
       folly::dynamic::object()("uri", source.uri)("width", source.width)("height", source.height);
 
   folly::dynamic eventData = folly::dynamic::object()("target", tag)("source", imageSource);
+  if (!error.empty()) {
+    eventData["error"] = winrt::to_string(error);
+  }
   GetReactContext().DispatchEvent(tag, eventName, std::move(eventData));
 }
 

diff --git a/vnext/Microsoft.ReactNative/Views/Image/ImageViewManager.h b/vnext/Microsoft.ReactNative/Views/Image/ImageViewManager.h
@@ -18,7 +18,11 @@ class ImageViewManager : public FrameworkElementViewManager {
       const winrt::Microsoft::ReactNative::IJSValueWriter &writer) const override;
   void GetNativeProps(const winrt::Microsoft::ReactNative::IJSValueWriter &writer) const override;
   ShadowNode *createShadow() const override;
-  void EmitImageEvent(xaml::Controls::Grid grid, const char *eventName, ReactImageSource &source);
+  void EmitImageEvent(
+      xaml::Controls::Grid grid,
+      const char *eventName,
+      ReactImageSource &source,
+      const winrt::hstring &errorMessage = L"");
   void SetLayoutProps(
       ShadowNodeBase &nodeToUpdate,
       const XamlView &viewToUpdate,

diff --git a/vnext/Microsoft.ReactNative/Views/Image/ReactImage.cpp b/vnext/Microsoft.ReactNative/Views/Image/ReactImage.cpp
@@ -53,7 +53,7 @@ winrt::Size ReactImage::ArrangeOverride(winrt::Size finalSize) {
   return finalSize;
 }
 
-winrt::event_token ReactImage::OnLoadEnd(winrt::EventHandler<bool> const &handler) {
+winrt::event_token ReactImage::OnLoadEnd(winrt::EventHandler<winrt::hstring> const &handler) {
   return m_onLoadEndEvent.add(handler);
 }
 
@@ -143,7 +143,7 @@ winrt::Stretch ReactImage::ResizeModeToStretch(winrt::Size size) {
 
 void ReactImage::Source(ReactImageSource source) {
   if (source.uri.length() == 0) {
-    m_onLoadEndEvent(*this, false);
+    m_onLoadEndEvent(*this, L"Empty URI");
     return;
   }
 
@@ -166,8 +166,8 @@ void ReactImage::Source(ReactImageSource source) {
     m_imageSource = source;
 
     SetBackground(true);
-  } catch (winrt::hresult_error const &) {
-    m_onLoadEndEvent(*this, false);
+  } catch (winrt::hresult_error const &err) {
+    m_onLoadEndEvent(*this, L"Failed to initialize ReactImage from source: " + err.message());
   }
 }
 
@@ -181,25 +181,33 @@ std::wstring GetUriFromImage(const winrt::Uri &uri) {
 }
 
 template <typename TImage>
-void ImageFailed(const TImage &image, const xaml::ExceptionRoutedEventArgs &args) {
+std::wstring ImageFailed(const TImage &image, const xaml::ExceptionRoutedEventArgs &args) {
+  std::wstring error{L"Failed to load image (" + args.ErrorMessage() + L")"};
 #ifdef DEBUG
-  cdebug << L"Failed to load image " << GetUriFromImage(image) << L" (" << args.ErrorMessage().c_str() << L")\n";
+  cdebug << error << L": " << GetUriFromImage(image) << L"\n";
 #endif
+  return error;
 }
 
 // TSourceFailedEventArgs can be either LoadedImageSourceLoadCompletedEventArgs or
 // SvgImageSourceFailedEventArgs, because they both have Status() properties
 // and the type of status are both enums with the same meaning
 // See LoadedImageSourceLoadStatus and SvgImageSourceLoadStatus.
 template <typename TImage, typename TSourceFailedEventArgs>
-void ImageFailed(const TImage &image, const TSourceFailedEventArgs &args) {
+std::wstring ImageFailed(const TImage &image, const TSourceFailedEventArgs &args) {
   // https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.media.loadedimagesourceloadstatus
-#ifdef DEBUG
   constexpr std::wstring_view statusNames[] = {L"Success", L"NetworkError", L"InvalidFormat", L"Other"};
   const auto status = (int)args.Status();
+
+  std::wstringstream error{L"Failed to load image"};
+  if (0 <= status && status < ARRAYSIZE(statusNames)) {
+    error << L" (" << statusNames[status] << L")";
+  }
+#ifdef DEBUG
   assert(0 <= status && status < ARRAYSIZE(statusNames));
-  cdebug << L"Failed to load image " << GetUriFromImage(image) << L" (" << statusNames[status] << L")\n";
+  cdebug << error.str() << L": " << GetUriFromImage(image) << L")\n";
 #endif
+  return error.str();
 }
 
 winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) {
@@ -224,15 +232,15 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) {
       // Fire failed load event if we're not loading from URI and the memory stream is null.
       if (!memoryStream) {
         if (auto strong_this{weak_this.get()}) {
-          strong_this->m_onLoadEndEvent(*strong_this, false);
+          strong_this->m_onLoadEndEvent(*strong_this, L"Failed to get image memory stream");
         }
         co_return;
       }
     }
-  } catch (winrt::hresult_error const &) {
+  } catch (winrt::hresult_error const &err) {
     const auto strong_this{weak_this.get()};
     if (strong_this && fireLoadEndEvent) {
-      strong_this->m_onLoadEndEvent(*strong_this, false);
+      strong_this->m_onLoadEndEvent(*strong_this, L"Failed to set background: " + err.message());
     }
     co_return;
   }
@@ -269,7 +277,7 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) {
                 winrt::LoadedImageSurface const & /*sender*/,
                 winrt::LoadedImageSourceLoadCompletedEventArgs const &args) {
               if (auto strong_this{weak_this.get()}) {
-                bool succeeded{false};
+                std::wstring error;
                 if (args.Status() == winrt::LoadedImageSourceLoadStatus::Success) {
                   winrt::Size size{surface.DecodedPhysicalSize()};
                   strong_this->m_imageSource.height = size.Height;
@@ -290,13 +298,12 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) {
                   compositionBrush->TintColor(strong_this->m_tintColor);
 
                   strong_this->Background(compositionBrush.as<winrt::XamlCompositionBrushBase>());
-                  succeeded = true;
                 } else {
-                  ImageFailed(uri, args);
+                  error = ImageFailed(uri, args);
                 }
 
                 if (fireLoadEndEvent) {
-                  strong_this->m_onLoadEndEvent(*strong_this, succeeded);
+                  strong_this->m_onLoadEndEvent(*strong_this, error);
                 }
 
                 strong_this->m_sizeChangedRevoker.revoke();
@@ -321,17 +328,17 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) {
               svgImageSource.Opened(winrt::auto_revoke, [weak_this, fireLoadEndEvent](const auto &, const auto &) {
                 auto strong_this{weak_this.get()};
                 if (strong_this && fireLoadEndEvent) {
-                  strong_this->m_onLoadEndEvent(*strong_this, true);
+                  strong_this->m_onLoadEndEvent(*strong_this, L"");
                 }
               });
 
           strong_this->m_svgImageSourceOpenFailedRevoker = svgImageSource.OpenFailed(
               winrt::auto_revoke, [weak_this, fireLoadEndEvent, svgImageSource](const auto &, const auto &args) {
                 auto strong_this{weak_this.get()};
+                auto error = ImageFailed(svgImageSource, args);
                 if (strong_this && fireLoadEndEvent) {
-                  strong_this->m_onLoadEndEvent(*strong_this, false);
+                  strong_this->m_onLoadEndEvent(*strong_this, error);
                 }
-                ImageFailed(svgImageSource, args);
               });
 
           imageBrush.ImageSource(svgImageSource);
@@ -376,7 +383,7 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) {
                     imageBrush.Stretch(strong_this->ResizeModeToStretch());
                   }
 
-                  strong_this->m_onLoadEndEvent(*strong_this, true);
+                  strong_this->m_onLoadEndEvent(*strong_this, L"");
                 }
               });
 
@@ -385,11 +392,11 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) {
               [imageBrush, weak_this, fireLoadEndEvent, bitmapImage](const auto &, const auto &args) {
                 imageBrush.Opacity(1);
 
+                auto error = ImageFailed(bitmapImage, args);
                 auto strong_this{weak_this.get()};
                 if (strong_this && fireLoadEndEvent) {
-                  strong_this->m_onLoadEndEvent(*strong_this, false);
+                  strong_this->m_onLoadEndEvent(*strong_this, error);
                 }
-                ImageFailed(bitmapImage, args);
               });
 
           imageBrush.ImageSource(bitmapImage);

diff --git a/vnext/Microsoft.ReactNative/Views/Image/ReactImage.h b/vnext/Microsoft.ReactNative/Views/Image/ReactImage.h
@@ -27,7 +27,7 @@ struct ReactImage : xaml::Controls::GridT<ReactImage> {
   xaml::Automation::Peers::AutomationPeer OnCreateAutomationPeer();
 
   // Events
-  winrt::event_token OnLoadEnd(winrt::Windows::Foundation::EventHandler<bool> const &handler);
+  winrt::event_token OnLoadEnd(winrt::Windows::Foundation::EventHandler<winrt::hstring> const &handler);
   void OnLoadEnd(winrt::event_token const &token) noexcept;
 
   // Public Properties
@@ -63,7 +63,7 @@ struct ReactImage : xaml::Controls::GridT<ReactImage> {
   facebook::react::ImageResizeMode m_resizeMode{facebook::react::ImageResizeMode::Contain};
   winrt::Windows::UI::Color m_tintColor{winrt::Colors::Transparent()};
 
-  winrt::event<winrt::Windows::Foundation::EventHandler<bool>> m_onLoadEndEvent;
+  winrt::event<winrt::Windows::Foundation::EventHandler<winrt::hstring>> m_onLoadEndEvent;
   xaml::FrameworkElement::SizeChanged_revoker m_sizeChangedRevoker;
   xaml::Media::LoadedImageSurface::LoadCompleted_revoker m_surfaceLoadedRevoker;
   xaml::Media::Imaging::BitmapImage::ImageOpened_revoker m_bitmapImageOpened;