From cd0f99c646ee667c0882f909a216967b636146d9 Mon Sep 17 00:00:00 2001
From: Kevin Reid <kpreid@switchb.org>
Date: Wed, 16 Aug 2023 12:10:04 -0700
Subject: [PATCH 1/6] Add details to `InstanceError` and `CreateSurfaceError`.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This will allow developers, particularly those in communication with
users with hardware they don't own, to diagnose “why won't this work
on my machine” better, without needing to obtain logs in addition to
the error value.

Note that this is a semver-breaking change because `InstanceError` and
`CreateSurfaceError` no longer implement `PartialEq`. This could be
avoided by pointer comparison of the internal `Arc`, but I think it is
more typical Rust to not implement `PartialEq` for error types unless
there is a strong motivation to and their contents are tightly
controlled.
---
 CHANGELOG.md                       |  3 ++
 wgpu-hal/examples/halmark/main.rs  |  8 +--
 wgpu-hal/src/auxil/dxgi/factory.rs | 42 ++++++++++------
 wgpu-hal/src/dx11/instance.rs      |  7 ++-
 wgpu-hal/src/dx12/instance.rs      |  8 ++-
 wgpu-hal/src/gles/adapter.rs       | 45 +++++++++--------
 wgpu-hal/src/gles/egl.rs           | 43 ++++++++++------
 wgpu-hal/src/gles/web.rs           | 16 +++---
 wgpu-hal/src/lib.rs                | 41 +++++++++++++--
 wgpu-hal/src/metal/mod.rs          |  4 +-
 wgpu-hal/src/vulkan/instance.rs    | 67 ++++++++++++++----------
 wgpu/src/backend/direct.rs         | 10 +---
 wgpu/src/backend/web.rs            | 17 +++++--
 wgpu/src/lib.rs                    | 81 +++++++++++++++++++++++-------
 14 files changed, 268 insertions(+), 124 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8da75f85c5..815d774ebe 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -70,9 +70,12 @@ By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402)
 
 ### Changes
 
+#### General
+
 - Omit texture store bound checks since they are no-ops if out of bounds on all APIs. By @teoxoy in [#3975](https://github.com/gfx-rs/wgpu/pull/3975)
 - Validate `DownlevelFlags::READ_ONLY_DEPTH_STENCIL`. By @teoxoy in [#4031](https://github.com/gfx-rs/wgpu/pull/4031)
 - Add validation in accordance with WebGPU `setViewport` valid usage for `x`, `y` and `this.[[attachment_size]]`. By @James2022-rgb in [#4058](https://github.com/gfx-rs/wgpu/pull/4058)
+- `wgpu::CreateSurfaceError` now gives details of the failure, but no longer implements `PartialEq`. By @kpreid in [#4066](https://github.com/gfx-rs/wgpu/pull/4066)
 
 #### Vulkan
 
diff --git a/wgpu-hal/examples/halmark/main.rs b/wgpu-hal/examples/halmark/main.rs
index c6b739bf17..5518cdaf4b 100644
--- a/wgpu-hal/examples/halmark/main.rs
+++ b/wgpu-hal/examples/halmark/main.rs
@@ -86,7 +86,7 @@ struct Example<A: hal::Api> {
 }
 
 impl<A: hal::Api> Example<A> {
-    fn init(window: &winit::window::Window) -> Result<Self, hal::InstanceError> {
+    fn init(window: &winit::window::Window) -> Result<Self, Box<dyn std::error::Error>> {
         let instance_desc = hal::InstanceDescriptor {
             name: "example",
             flags: if cfg!(debug_assertions) {
@@ -108,13 +108,13 @@ impl<A: hal::Api> Example<A> {
         let (adapter, capabilities) = unsafe {
             let mut adapters = instance.enumerate_adapters();
             if adapters.is_empty() {
-                return Err(hal::InstanceError);
+                return Err("no adapters found".into());
             }
             let exposed = adapters.swap_remove(0);
             (exposed.adapter, exposed.capabilities)
         };
-        let surface_caps =
-            unsafe { adapter.surface_capabilities(&surface) }.ok_or(hal::InstanceError)?;
+        let surface_caps = unsafe { adapter.surface_capabilities(&surface) }
+            .ok_or("failed to get surface capabilities")?;
         log::info!("Surface caps: {:#?}", surface_caps);
 
         let hal::OpenDevice { device, mut queue } = unsafe {
diff --git a/wgpu-hal/src/auxil/dxgi/factory.rs b/wgpu-hal/src/auxil/dxgi/factory.rs
index 123ca4933e..b6abe9b3e8 100644
--- a/wgpu-hal/src/auxil/dxgi/factory.rs
+++ b/wgpu-hal/src/auxil/dxgi/factory.rs
@@ -96,7 +96,9 @@ pub fn create_factory(
     required_factory_type: DxgiFactoryType,
     instance_flags: crate::InstanceFlags,
 ) -> Result<(d3d12::DxgiLib, d3d12::DxgiFactory), crate::InstanceError> {
-    let lib_dxgi = d3d12::DxgiLib::new().map_err(|_| crate::InstanceError)?;
+    let lib_dxgi = d3d12::DxgiLib::new().map_err(|e| {
+        crate::InstanceError::with_source(String::from("failed to load dxgi.dll"), e)
+    })?;
 
     let mut factory_flags = d3d12::FactoryCreationFlags::empty();
 
@@ -128,18 +130,22 @@ pub fn create_factory(
             Ok(factory) => Some(factory),
             // We hard error here as we _should have_ been able to make a factory4 but couldn't.
             Err(err) => {
-                log::error!("Failed to create IDXGIFactory4: {}", err);
-                return Err(crate::InstanceError);
+                return Err(crate::InstanceError::with_source(
+                    String::from("failed to create IDXGIFactory4"),
+                    err,
+                ));
             }
         },
         // If we require factory4, hard error.
         Err(err) if required_factory_type == DxgiFactoryType::Factory4 => {
-            log::error!("IDXGIFactory1 creation function not found: {:?}", err);
-            return Err(crate::InstanceError);
+            return Err(crate::InstanceError::with_source(
+                String::from("IDXGIFactory1 creation function not found"),
+                err,
+            ));
         }
         // If we don't print it to info as all win7 will hit this case.
         Err(err) => {
-            log::info!("IDXGIFactory1 creation function not found: {:?}", err);
+            log::info!("IDXGIFactory1 creation function not found: {err:?}");
             None
         }
     };
@@ -153,8 +159,10 @@ pub fn create_factory(
             }
             // If we require factory6, hard error.
             Err(err) if required_factory_type == DxgiFactoryType::Factory6 => {
-                log::warn!("Failed to cast IDXGIFactory4 to IDXGIFactory6: {:?}", err);
-                return Err(crate::InstanceError);
+                return Err(crate::InstanceError::with_source(
+                    format!("failed to cast IDXGIFactory4 to IDXGIFactory6"),
+                    err,
+                ));
             }
             // If we don't print it to info.
             Err(err) => {
@@ -169,14 +177,18 @@ pub fn create_factory(
         Ok(pair) => match pair.into_result() {
             Ok(factory) => factory,
             Err(err) => {
-                log::error!("Failed to create IDXGIFactory1: {}", err);
-                return Err(crate::InstanceError);
+                return Err(crate::InstanceError::with_source(
+                    format!("failed to create IDXGIFactory1"),
+                    err,
+                ));
             }
         },
         // We always require at least factory1, so hard error
         Err(err) => {
-            log::error!("IDXGIFactory1 creation function not found: {:?}", err);
-            return Err(crate::InstanceError);
+            return Err(crate::InstanceError::with_source(
+                format!("IDXGIFactory1 creation function not found"),
+                err,
+            ));
         }
     };
 
@@ -188,8 +200,10 @@ pub fn create_factory(
         }
         // If we require factory2, hard error.
         Err(err) if required_factory_type == DxgiFactoryType::Factory2 => {
-            log::warn!("Failed to cast IDXGIFactory1 to IDXGIFactory2: {:?}", err);
-            return Err(crate::InstanceError);
+            return Err(crate::InstanceError::with_source(
+                format!("failed to cast IDXGIFactory1 to IDXGIFactory2"),
+                err,
+            ));
         }
         // If we don't print it to info.
         Err(err) => {
diff --git a/wgpu-hal/src/dx11/instance.rs b/wgpu-hal/src/dx11/instance.rs
index 1d8c2b51a2..e7a4e2e705 100644
--- a/wgpu-hal/src/dx11/instance.rs
+++ b/wgpu-hal/src/dx11/instance.rs
@@ -8,10 +8,13 @@ impl crate::Instance<super::Api> for super::Instance {
         };
 
         if !enable_dx11 {
-            return Err(crate::InstanceError);
+            return Err(crate::InstanceError::new(String::from(
+                "DX11 support is unstable; set WGPU_UNSTABLE_DX11_BACKEND=1 to enable anyway",
+            )));
         }
 
-        let lib_d3d11 = super::library::D3D11Lib::new().ok_or(crate::InstanceError)?;
+        let lib_d3d11 = super::library::D3D11Lib::new()
+            .ok_or_else(|| crate::InstanceError::new(String::from("failed to load d3d11.dll")))?;
 
         let (lib_dxgi, factory) = auxil::dxgi::factory::create_factory(
             auxil::dxgi::factory::DxgiFactoryType::Factory1,
diff --git a/wgpu-hal/src/dx12/instance.rs b/wgpu-hal/src/dx12/instance.rs
index 208d2179f7..32d6f1690c 100644
--- a/wgpu-hal/src/dx12/instance.rs
+++ b/wgpu-hal/src/dx12/instance.rs
@@ -12,7 +12,9 @@ impl Drop for super::Instance {
 
 impl crate::Instance<super::Api> for super::Instance {
     unsafe fn init(desc: &crate::InstanceDescriptor) -> Result<Self, crate::InstanceError> {
-        let lib_main = d3d12::D3D12Lib::new().map_err(|_| crate::InstanceError)?;
+        let lib_main = d3d12::D3D12Lib::new().map_err(|e| {
+            crate::InstanceError::with_source(String::from("failed to load d3d12.dll"), e)
+        })?;
 
         if desc.flags.contains(crate::InstanceFlags::VALIDATION) {
             // Enable debug layer
@@ -95,7 +97,9 @@ impl crate::Instance<super::Api> for super::Instance {
                 supports_allow_tearing: self.supports_allow_tearing,
                 swap_chain: None,
             }),
-            _ => Err(crate::InstanceError),
+            _ => Err(crate::InstanceError::new(format!(
+                "window handle {window_handle:?} is not a Win32 handle"
+            ))),
         }
     }
     unsafe fn destroy_surface(&self, _surface: super::Surface) {
diff --git a/wgpu-hal/src/gles/adapter.rs b/wgpu-hal/src/gles/adapter.rs
index ca75bd09ef..febf090ee0 100644
--- a/wgpu-hal/src/gles/adapter.rs
+++ b/wgpu-hal/src/gles/adapter.rs
@@ -43,8 +43,9 @@ impl super::Adapter {
                     src = &src[pos + es_sig.len()..];
                 }
                 None => {
-                    log::warn!("ES not found in '{}'", src);
-                    return Err(crate::InstanceError);
+                    return Err(crate::InstanceError::new(format!(
+                        "OpenGL version {src:?} does not contain 'ES'"
+                    )));
                 }
             }
         };
@@ -86,10 +87,9 @@ impl super::Adapter {
                 },
                 minor,
             )),
-            _ => {
-                log::warn!("Unable to extract the version from '{}'", version);
-                Err(crate::InstanceError)
-            }
+            _ => Err(crate::InstanceError::new(format!(
+                "unable to extract OpenGL version from {version:?}"
+            ))),
         }
     }
 
@@ -974,27 +974,30 @@ mod tests {
 
     #[test]
     fn test_version_parse() {
-        let error = Err(crate::InstanceError);
-        assert_eq!(Adapter::parse_version("1"), error);
-        assert_eq!(Adapter::parse_version("1."), error);
-        assert_eq!(Adapter::parse_version("1 h3l1o. W0rld"), error);
-        assert_eq!(Adapter::parse_version("1. h3l1o. W0rld"), error);
-        assert_eq!(Adapter::parse_version("1.2.3"), error);
-        assert_eq!(Adapter::parse_version("OpenGL ES 3.1"), Ok((3, 1)));
+        Adapter::parse_version("1").unwrap_err();
+        Adapter::parse_version("1.").unwrap_err();
+        Adapter::parse_version("1 h3l1o. W0rld").unwrap_err();
+        Adapter::parse_version("1. h3l1o. W0rld").unwrap_err();
+        Adapter::parse_version("1.2.3").unwrap_err();
+
+        assert_eq!(Adapter::parse_version("OpenGL ES 3.1").unwrap(), (3, 1));
+        assert_eq!(
+            Adapter::parse_version("OpenGL ES 2.0 Google Nexus").unwrap(),
+            (2, 0)
+        );
+        assert_eq!(Adapter::parse_version("GLSL ES 1.1").unwrap(), (1, 1));
         assert_eq!(
-            Adapter::parse_version("OpenGL ES 2.0 Google Nexus"),
-            Ok((2, 0))
+            Adapter::parse_version("OpenGL ES GLSL ES 3.20").unwrap(),
+            (3, 2)
         );
-        assert_eq!(Adapter::parse_version("GLSL ES 1.1"), Ok((1, 1)));
-        assert_eq!(Adapter::parse_version("OpenGL ES GLSL ES 3.20"), Ok((3, 2)));
         assert_eq!(
             // WebGL 2.0 should parse as OpenGL ES 3.0
-            Adapter::parse_version("WebGL 2.0 (OpenGL ES 3.0 Chromium)"),
-            Ok((3, 0))
+            Adapter::parse_version("WebGL 2.0 (OpenGL ES 3.0 Chromium)").unwrap(),
+            (3, 0)
         );
         assert_eq!(
-            Adapter::parse_version("WebGL GLSL ES 3.00 (OpenGL ES GLSL ES 3.0 Chromium)"),
-            Ok((3, 0))
+            Adapter::parse_version("WebGL GLSL ES 3.00 (OpenGL ES GLSL ES 3.0 Chromium)").unwrap(),
+            (3, 0)
         );
     }
 }
diff --git a/wgpu-hal/src/gles/egl.rs b/wgpu-hal/src/gles/egl.rs
index b904dffee9..f0f315f350 100644
--- a/wgpu-hal/src/gles/egl.rs
+++ b/wgpu-hal/src/gles/egl.rs
@@ -283,7 +283,10 @@ fn choose_config(
         }
     }
 
-    Err(crate::InstanceError)
+    // TODO: include diagnostic details that are currently logged
+    Err(crate::InstanceError::new(String::from(
+        "unable to find an acceptable EGL framebuffer configuration",
+    )))
 }
 
 fn gl_debug_message_callback(source: u32, gltype: u32, id: u32, severity: u32, message: &str) {
@@ -495,7 +498,12 @@ impl Inner {
         display: khronos_egl::Display,
         force_gles_minor_version: wgt::Gles3MinorVersion,
     ) -> Result<Self, crate::InstanceError> {
-        let version = egl.initialize(display).map_err(|_| crate::InstanceError)?;
+        let version = egl.initialize(display).map_err(|e| {
+            crate::InstanceError::with_source(
+                String::from("failed to initialize EGL display connection"),
+                e,
+            )
+        })?;
         let vendor = egl
             .query_string(Some(display), khronos_egl::VENDOR)
             .unwrap();
@@ -599,8 +607,10 @@ impl Inner {
         let context = match egl.create_context(display, config, None, &context_attributes) {
             Ok(context) => context,
             Err(e) => {
-                log::warn!("unable to create GLES 3.x context: {:?}", e);
-                return Err(crate::InstanceError);
+                return Err(crate::InstanceError::with_source(
+                    String::from("unable to create GLES 3.x context"),
+                    e,
+                ));
             }
         };
 
@@ -623,8 +633,10 @@ impl Inner {
             egl.create_pbuffer_surface(display, config, &attributes)
                 .map(Some)
                 .map_err(|e| {
-                    log::warn!("Error in create_pbuffer_surface: {:?}", e);
-                    crate::InstanceError
+                    crate::InstanceError::with_source(
+                        String::from("error in create_pbuffer_surface"),
+                        e,
+                    )
                 })?
         };
 
@@ -734,8 +746,10 @@ impl crate::Instance<super::Api> for Instance {
         let egl = match egl_result {
             Ok(egl) => Arc::new(egl),
             Err(e) => {
-                log::info!("Unable to open libEGL: {:?}", e);
-                return Err(crate::InstanceError);
+                return Err(crate::InstanceError::with_source(
+                    String::from("unable to open libEGL"),
+                    e,
+                ));
             }
         };
 
@@ -899,8 +913,9 @@ impl crate::Instance<super::Api> for Instance {
                 };
 
                 if ret != 0 {
-                    log::error!("Error returned from ANativeWindow_setBuffersGeometry");
-                    return Err(crate::InstanceError);
+                    return Err(crate::InstanceError::new(format(
+                        "error {ret} returned from ANativeWindow_setBuffersGeometry",
+                    )));
                 }
             }
             #[cfg(not(target_os = "emscripten"))]
@@ -938,8 +953,7 @@ impl crate::Instance<super::Api> for Instance {
                         Arc::clone(&inner.egl.instance),
                         display,
                         inner.force_gles_minor_version,
-                    )
-                    .map_err(|_| crate::InstanceError)?;
+                    )?;
 
                     let old_inner = std::mem::replace(inner.deref_mut(), new_inner);
                     inner.wl_display = Some(display_handle.display);
@@ -950,8 +964,9 @@ impl crate::Instance<super::Api> for Instance {
             #[cfg(target_os = "emscripten")]
             (Rwh::Web(_), _) => {}
             other => {
-                log::error!("Unsupported window: {:?}", other);
-                return Err(crate::InstanceError);
+                return Err(crate::InstanceError::new(format!(
+                    "unsupported window: {other:?}"
+                )));
             }
         };
 
diff --git a/wgpu-hal/src/gles/web.rs b/wgpu-hal/src/gles/web.rs
index 49bc5656de..6937acdb3c 100644
--- a/wgpu-hal/src/gles/web.rs
+++ b/wgpu-hal/src/gles/web.rs
@@ -65,14 +65,16 @@ impl Instance {
                 // “not supported” could include “insufficient GPU resources” or “the GPU process
                 // previously crashed”. So, we must return it as an `Err` since it could occur
                 // for circumstances outside the application author's control.
-                return Err(crate::InstanceError);
+                return Err(crate::InstanceError::new(String::from(
+                    "canvas.getContext() returned null; webgl2 not available or canvas already in use"
+                )));
             }
             Err(js_error) => {
                 // <https://html.spec.whatwg.org/multipage/canvas.html#dom-canvas-getcontext>
-                // A thrown exception indicates misuse of the canvas state. Ideally we wouldn't
-                // panic in this case, but for now, `InstanceError` conveys no detail, so it
-                // is more informative to panic with a specific message.
-                panic!("canvas.getContext() threw {js_error:?}")
+                // A thrown exception indicates misuse of the canvas state.
+                return Err(crate::InstanceError::new(format!(
+                    "canvas.getContext() threw exception {js_error:?}",
+                )));
             }
         };
 
@@ -153,7 +155,9 @@ impl crate::Instance<super::Api> for Instance {
 
             self.create_surface_from_canvas(canvas)
         } else {
-            Err(crate::InstanceError)
+            Err(crate::InstanceError::new(format!(
+                "window handle {window_handle:?} is not a web handle"
+            )))
         }
     }
 
diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs
index 020c665709..4bff6b8d8f 100644
--- a/wgpu-hal/src/lib.rs
+++ b/wgpu-hal/src/lib.rs
@@ -90,7 +90,7 @@ use std::{
     num::NonZeroU32,
     ops::{Range, RangeInclusive},
     ptr::NonNull,
-    sync::atomic::AtomicBool,
+    sync::{atomic::AtomicBool, Arc},
 };
 
 use bitflags::bitflags;
@@ -152,9 +152,42 @@ pub enum SurfaceError {
     Other(&'static str),
 }
 
-#[derive(Clone, Debug, Eq, PartialEq, Error)]
-#[error("Not supported")]
-pub struct InstanceError;
+/// Error occurring while trying to create an instance, or create a surface from an instance;
+/// typically relating to the state of the underlying graphics API or hardware.
+#[derive(Clone, Debug, Error)]
+#[error("{message}")]
+pub struct InstanceError {
+    /// These errors are very platform specific, so do not attempt to encode them as an enum.
+    ///
+    /// This message should describe the problem in sufficient detail to be useful for a
+    /// user-to-developer “why won't this work on my machine” bug report, and otherwise follow
+    /// <https://rust-lang.github.io/api-guidelines/interoperability.html#error-types-are-meaningful-and-well-behaved-c-good-err>.
+    message: String,
+
+    /// Underlying error value, if any is available.
+    #[source]
+    source: Option<Arc<dyn std::error::Error + Send + Sync + 'static>>,
+}
+
+impl InstanceError {
+    #[allow(dead_code)] // may be unused on some platforms
+    pub(crate) fn new(message: String) -> Self {
+        Self {
+            message,
+            source: None,
+        }
+    }
+    #[allow(dead_code)] // may be unused on some platforms
+    pub(crate) fn with_source(
+        message: String,
+        source: impl std::error::Error + Send + Sync + 'static,
+    ) -> Self {
+        Self {
+            message,
+            source: Some(Arc::new(source)),
+        }
+    }
+}
 
 pub trait Api: Clone + Sized {
     type Instance: Instance<Self>;
diff --git a/wgpu-hal/src/metal/mod.rs b/wgpu-hal/src/metal/mod.rs
index 3a8ebc5570..76f57002ff 100644
--- a/wgpu-hal/src/metal/mod.rs
+++ b/wgpu-hal/src/metal/mod.rs
@@ -100,7 +100,9 @@ impl crate::Instance<Api> for Instance {
             raw_window_handle::RawWindowHandle::AppKit(handle) => Ok(unsafe {
                 Surface::from_view(handle.ns_view, Some(&self.managed_metal_layer_delegate))
             }),
-            _ => Err(crate::InstanceError),
+            _ => Err(crate::InstanceError::new(format!(
+                "window handle {window_handle:?} is not a Metal-compatible handle"
+            ))),
         }
     }
 
diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs
index 4fa4a3e27d..18b141a070 100644
--- a/wgpu-hal/src/vulkan/instance.rs
+++ b/wgpu-hal/src/vulkan/instance.rs
@@ -194,8 +194,10 @@ impl super::Instance {
         let instance_extensions = entry
             .enumerate_instance_extension_properties(None)
             .map_err(|e| {
-                log::info!("enumerate_instance_extension_properties: {:?}", e);
-                crate::InstanceError
+                crate::InstanceError::with_source(
+                    String::from("enumerate_instance_extension_properties() failed"),
+                    e,
+                )
             })?;
 
         // Check our extensions against the available extensions
@@ -366,8 +368,9 @@ impl super::Instance {
         window: vk::Window,
     ) -> Result<super::Surface, crate::InstanceError> {
         if !self.shared.extensions.contains(&khr::XlibSurface::name()) {
-            log::warn!("Vulkan driver does not support VK_KHR_xlib_surface");
-            return Err(crate::InstanceError);
+            return Err(crate::InstanceError::new(String::from(
+                "Vulkan driver does not support VK_KHR_xlib_surface",
+            )));
         }
 
         let surface = {
@@ -391,8 +394,9 @@ impl super::Instance {
         window: vk::xcb_window_t,
     ) -> Result<super::Surface, crate::InstanceError> {
         if !self.shared.extensions.contains(&khr::XcbSurface::name()) {
-            log::warn!("Vulkan driver does not support VK_KHR_xcb_surface");
-            return Err(crate::InstanceError);
+            return Err(crate::InstanceError::new(String::from(
+                "Vulkan driver does not support VK_KHR_xcb_surface",
+            )));
         }
 
         let surface = {
@@ -420,8 +424,9 @@ impl super::Instance {
             .extensions
             .contains(&khr::WaylandSurface::name())
         {
-            log::debug!("Vulkan driver does not support VK_KHR_wayland_surface");
-            return Err(crate::InstanceError);
+            return Err(crate::InstanceError::new(String::from(
+                "Vulkan driver does not support VK_KHR_wayland_surface",
+            )));
         }
 
         let surface = {
@@ -447,8 +452,9 @@ impl super::Instance {
             .extensions
             .contains(&khr::AndroidSurface::name())
         {
-            log::warn!("Vulkan driver does not support VK_KHR_android_surface");
-            return Err(crate::InstanceError);
+            return Err(crate::InstanceError::new(String::from(
+                "Vulkan driver does not support VK_KHR_android_surface",
+            )));
         }
 
         let surface = {
@@ -470,8 +476,9 @@ impl super::Instance {
         hwnd: *mut c_void,
     ) -> Result<super::Surface, crate::InstanceError> {
         if !self.shared.extensions.contains(&khr::Win32Surface::name()) {
-            log::debug!("Vulkan driver does not support VK_KHR_win32_surface");
-            return Err(crate::InstanceError);
+            return Err(crate::InstanceError::new(String::from(
+                "Vulkan driver does not support VK_KHR_win32_surface",
+            )));
         }
 
         let surface = {
@@ -496,8 +503,9 @@ impl super::Instance {
         view: *mut c_void,
     ) -> Result<super::Surface, crate::InstanceError> {
         if !self.shared.extensions.contains(&ext::MetalSurface::name()) {
-            log::warn!("Vulkan driver does not support VK_EXT_metal_surface");
-            return Err(crate::InstanceError);
+            return Err(crate::InstanceError::new(String::from(
+                "Vulkan driver does not support VK_EXT_metal_surface",
+            )));
         }
 
         let layer = unsafe {
@@ -546,20 +554,18 @@ impl crate::Instance<super::Api> for super::Instance {
     unsafe fn init(desc: &crate::InstanceDescriptor) -> Result<Self, crate::InstanceError> {
         use crate::auxil::cstr_from_bytes_until_nul;
 
-        let entry = match unsafe { ash::Entry::load() } {
-            Ok(entry) => entry,
-            Err(err) => {
-                log::info!("Missing Vulkan entry points: {:?}", err);
-                return Err(crate::InstanceError);
-            }
-        };
+        let entry = unsafe { ash::Entry::load() }.map_err(|err| {
+            crate::InstanceError::with_source(String::from("missing Vulkan entry points"), err)
+        })?;
         let driver_api_version = match entry.try_enumerate_instance_version() {
             // Vulkan 1.1+
             Ok(Some(version)) => version,
             Ok(None) => vk::API_VERSION_1_0,
             Err(err) => {
-                log::warn!("try_enumerate_instance_version: {:?}", err);
-                return Err(crate::InstanceError);
+                return Err(crate::InstanceError::with_source(
+                    String::from("try_enumerate_instance_version() failed"),
+                    err,
+                ));
             }
         };
 
@@ -590,7 +596,10 @@ impl crate::Instance<super::Api> for super::Instance {
 
         let instance_layers = entry.enumerate_instance_layer_properties().map_err(|e| {
             log::info!("enumerate_instance_layer_properties: {:?}", e);
-            crate::InstanceError
+            crate::InstanceError::with_source(
+                String::from("enumerate_instance_layer_properties() failed"),
+                e,
+            )
         })?;
 
         fn find_layer<'layers>(
@@ -682,8 +691,10 @@ impl crate::Instance<super::Api> for super::Instance {
                 .enabled_extension_names(&str_pointers[layers.len()..]);
 
             unsafe { entry.create_instance(&create_info, None) }.map_err(|e| {
-                log::warn!("create_instance: {:?}", e);
-                crate::InstanceError
+                crate::InstanceError::with_source(
+                    String::from("Entry::create_instance() failed"),
+                    e,
+                )
             })?
         };
 
@@ -739,7 +750,9 @@ impl crate::Instance<super::Api> for super::Instance {
             {
                 self.create_surface_from_view(handle.ui_view)
             }
-            (_, _) => Err(crate::InstanceError),
+            (_, _) => Err(crate::InstanceError::new(format!(
+                "window handle {window_handle:?} is not a Vulkan-compatible handle"
+            ))),
         }
     }
 
diff --git a/wgpu/src/backend/direct.rs b/wgpu/src/backend/direct.rs
index fca1d80c3c..8eec9adad5 100644
--- a/wgpu/src/backend/direct.rs
+++ b/wgpu/src/backend/direct.rs
@@ -244,10 +244,7 @@ impl Context {
         &self,
         canvas: web_sys::HtmlCanvasElement,
     ) -> Result<Surface, crate::CreateSurfaceError> {
-        let id = self
-            .0
-            .create_surface_webgl_canvas(canvas, ())
-            .map_err(|hal::InstanceError| crate::CreateSurfaceError {})?;
+        let id = self.0.create_surface_webgl_canvas(canvas, ())?;
         Ok(Surface {
             id,
             configured_device: Mutex::default(),
@@ -259,10 +256,7 @@ impl Context {
         &self,
         canvas: web_sys::OffscreenCanvas,
     ) -> Result<Surface, crate::CreateSurfaceError> {
-        let id = self
-            .0
-            .create_surface_webgl_offscreen_canvas(canvas, ())
-            .map_err(|hal::InstanceError| crate::CreateSurfaceError {})?;
+        let id = self.0.create_surface_webgl_offscreen_canvas(canvas, ())?;
         Ok(Surface {
             id,
             configured_device: Mutex::default(),
diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs
index 5fb8614a23..4851a7c9bf 100644
--- a/wgpu/src/backend/web.rs
+++ b/wgpu/src/backend/web.rs
@@ -827,13 +827,22 @@ impl Context {
                 // “not supported” could include “insufficient GPU resources” or “the GPU process
                 // previously crashed”. So, we must return it as an `Err` since it could occur
                 // for circumstances outside the application author's control.
-                return Err(crate::CreateSurfaceError {});
+                return Err(crate::CreateSurfaceError {
+                    inner: crate::CreateSurfaceErrorKind::Web(
+                        String::from(
+                            "canvas.getContext() returned null; webgpu not available or canvas already in use"
+                        )
+                    )
+                });
             }
             Err(js_error) => {
                 // <https://html.spec.whatwg.org/multipage/canvas.html#dom-canvas-getcontext>
-                // A thrown exception indicates misuse of the canvas state. Ideally we wouldn't
-                // panic in this case ... TODO
-                panic!("canvas.getContext() threw {js_error:?}")
+                // A thrown exception indicates misuse of the canvas state.
+                return Err(crate::CreateSurfaceError {
+                    inner: crate::CreateSurfaceErrorKind::Web(format!(
+                        "canvas.getContext() threw exception {js_error:?}",
+                    )),
+                });
             }
         };
 
diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs
index 1c3e1a58b5..f3028fc1e9 100644
--- a/wgpu/src/lib.rs
+++ b/wgpu/src/lib.rs
@@ -15,8 +15,7 @@ mod macros;
 use std::{
     any::Any,
     borrow::Cow,
-    error,
-    fmt::{Debug, Display},
+    error, fmt,
     future::Future,
     marker::PhantomData,
     num::NonZeroU32,
@@ -1700,8 +1699,8 @@ pub enum SurfaceError {
 }
 static_assertions::assert_impl_all!(SurfaceError: Send, Sync);
 
-impl Display for SurfaceError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+impl fmt::Display for SurfaceError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{}", match self {
             Self::Timeout => "A timeout was encountered while trying to acquire the next frame",
             Self::Outdated => "The underlying surface has changed, and therefore the swap chain must be updated",
@@ -2744,8 +2743,8 @@ impl Drop for Device {
 pub struct RequestDeviceError;
 static_assertions::assert_impl_all!(RequestDeviceError: Send, Sync);
 
-impl Display for RequestDeviceError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+impl fmt::Display for RequestDeviceError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "Requesting a device failed")
     }
 }
@@ -2753,28 +2752,76 @@ impl Display for RequestDeviceError {
 impl error::Error for RequestDeviceError {}
 
 /// [`Instance::create_surface()`] or a related function failed.
-#[derive(Clone, PartialEq, Eq, Debug)]
+#[derive(Clone, Debug)]
 #[non_exhaustive]
 pub struct CreateSurfaceError {
-    // TODO: Report diagnostic clues
+    inner: CreateSurfaceErrorKind,
+}
+#[derive(Clone, Debug)]
+enum CreateSurfaceErrorKind {
+    /// Error from [`wgpu_hal`].
+    #[cfg(any(
+        not(target_arch = "wasm32"),
+        target_os = "emscripten",
+        feature = "webgl"
+    ))]
+    // must match dependency cfg
+    Hal(hal::InstanceError),
+
+    /// Error from WebGPU surface creation.
+    #[allow(dead_code)] // may be unused depending on target and features
+    Web(String),
 }
 static_assertions::assert_impl_all!(CreateSurfaceError: Send, Sync);
 
-impl Display for CreateSurfaceError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "Creating a surface failed")
+impl fmt::Display for CreateSurfaceError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match &self.inner {
+            #[cfg(any(
+                not(target_arch = "wasm32"),
+                target_os = "emscripten",
+                feature = "webgl"
+            ))]
+            CreateSurfaceErrorKind::Hal(e) => e.fmt(f),
+            CreateSurfaceErrorKind::Web(e) => e.fmt(f),
+        }
     }
 }
 
-impl error::Error for CreateSurfaceError {}
+impl error::Error for CreateSurfaceError {
+    fn source(&self) -> Option<&(dyn error::Error + 'static)> {
+        match &self.inner {
+            #[cfg(any(
+                not(target_arch = "wasm32"),
+                target_os = "emscripten",
+                feature = "webgl"
+            ))]
+            CreateSurfaceErrorKind::Hal(e) => e.source(),
+            CreateSurfaceErrorKind::Web(_) => None,
+        }
+    }
+}
+
+#[cfg(any(
+    not(target_arch = "wasm32"),
+    target_os = "emscripten",
+    feature = "webgl"
+))]
+impl From<hal::InstanceError> for CreateSurfaceError {
+    fn from(e: hal::InstanceError) -> Self {
+        Self {
+            inner: CreateSurfaceErrorKind::Hal(e),
+        }
+    }
+}
 
 /// Error occurred when trying to async map a buffer.
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct BufferAsyncError;
 static_assertions::assert_impl_all!(BufferAsyncError: Send, Sync);
 
-impl Display for BufferAsyncError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+impl fmt::Display for BufferAsyncError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "Error occurred when trying to async map a buffer")
     }
 }
@@ -4850,7 +4897,7 @@ impl<T> Copy for Id<T> {}
 
 #[cfg(feature = "expose-ids")]
 impl<T> Debug for Id<T> {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         f.debug_tuple("Id").field(&self.0).finish()
     }
 }
@@ -5150,8 +5197,8 @@ impl error::Error for Error {
     }
 }
 
-impl Display for Error {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             Error::OutOfMemory { .. } => f.write_str("Out of Memory"),
             Error::Validation { description, .. } => f.write_str(description),

From 41fe862daafc4bc7d8ba39b91f75782f50c35bac Mon Sep 17 00:00:00 2001
From: Kevin Reid <kpreid@switchb.org>
Date: Wed, 16 Aug 2023 14:04:52 -0700
Subject: [PATCH 2/6] Fix DXGI error type handling

---
 wgpu-hal/src/auxil/dxgi/factory.rs | 34 +++++++++++++++---------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/wgpu-hal/src/auxil/dxgi/factory.rs b/wgpu-hal/src/auxil/dxgi/factory.rs
index b6abe9b3e8..7ae6e745f0 100644
--- a/wgpu-hal/src/auxil/dxgi/factory.rs
+++ b/wgpu-hal/src/auxil/dxgi/factory.rs
@@ -130,10 +130,10 @@ pub fn create_factory(
             Ok(factory) => Some(factory),
             // We hard error here as we _should have_ been able to make a factory4 but couldn't.
             Err(err) => {
-                return Err(crate::InstanceError::with_source(
-                    String::from("failed to create IDXGIFactory4"),
-                    err,
-                ));
+                // err is a Cow<str>, not an Error implementor
+                return Err(crate::InstanceError::new(format!(
+                    "failed to create IDXGIFactory4: {err:?}"
+                )));
             }
         },
         // If we require factory4, hard error.
@@ -159,10 +159,10 @@ pub fn create_factory(
             }
             // If we require factory6, hard error.
             Err(err) if required_factory_type == DxgiFactoryType::Factory6 => {
-                return Err(crate::InstanceError::with_source(
-                    format!("failed to cast IDXGIFactory4 to IDXGIFactory6"),
-                    err,
-                ));
+                // err is a Cow<str>, not an Error implementor
+                return Err(crate::InstanceError::new(format!(
+                    "failed to cast IDXGIFactory4 to IDXGIFactory6: {err:?}"
+                )));
             }
             // If we don't print it to info.
             Err(err) => {
@@ -177,16 +177,16 @@ pub fn create_factory(
         Ok(pair) => match pair.into_result() {
             Ok(factory) => factory,
             Err(err) => {
-                return Err(crate::InstanceError::with_source(
-                    format!("failed to create IDXGIFactory1"),
-                    err,
-                ));
+                // err is a Cow<str>, not an Error implementor
+                return Err(crate::InstanceError::new(format!(
+                    "failed to create IDXGIFactory1: {err:?}"
+                )));
             }
         },
         // We always require at least factory1, so hard error
         Err(err) => {
             return Err(crate::InstanceError::with_source(
-                format!("IDXGIFactory1 creation function not found"),
+                String::from("IDXGIFactory1 creation function not found"),
                 err,
             ));
         }
@@ -200,10 +200,10 @@ pub fn create_factory(
         }
         // If we require factory2, hard error.
         Err(err) if required_factory_type == DxgiFactoryType::Factory2 => {
-            return Err(crate::InstanceError::with_source(
-                format!("failed to cast IDXGIFactory1 to IDXGIFactory2"),
-                err,
-            ));
+            // err is a Cow<str>, not an Error implementor
+            return Err(crate::InstanceError::new(format!(
+                "failed to cast IDXGIFactory1 to IDXGIFactory2: {err:?}"
+            )));
         }
         // If we don't print it to info.
         Err(err) => {

From 93f52c750531676a125925e54ae564e21fb14c1c Mon Sep 17 00:00:00 2001
From: Kevin Reid <kpreid@switchb.org>
Date: Wed, 16 Aug 2023 14:05:41 -0700
Subject: [PATCH 3/6] fix fmt::Debug path with expose-ids

---
 wgpu/src/lib.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs
index f3028fc1e9..94345f1adb 100644
--- a/wgpu/src/lib.rs
+++ b/wgpu/src/lib.rs
@@ -4896,7 +4896,7 @@ impl<T> Clone for Id<T> {
 impl<T> Copy for Id<T> {}
 
 #[cfg(feature = "expose-ids")]
-impl<T> Debug for Id<T> {
+impl<T> fmt::Debug for Id<T> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         f.debug_tuple("Id").field(&self.0).finish()
     }

From 42cecb624e242826d7da587775cf942205db4e16 Mon Sep 17 00:00:00 2001
From: Kevin Reid <kpreid@switchb.org>
Date: Wed, 16 Aug 2023 14:20:09 -0700
Subject: [PATCH 4/6] Fix typo affecting Android only

---
 wgpu-hal/src/gles/egl.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/wgpu-hal/src/gles/egl.rs b/wgpu-hal/src/gles/egl.rs
index f0f315f350..d6d3d621f9 100644
--- a/wgpu-hal/src/gles/egl.rs
+++ b/wgpu-hal/src/gles/egl.rs
@@ -913,7 +913,7 @@ impl crate::Instance<super::Api> for Instance {
                 };
 
                 if ret != 0 {
-                    return Err(crate::InstanceError::new(format(
+                    return Err(crate::InstanceError::new(format!(
                         "error {ret} returned from ANativeWindow_setBuffersGeometry",
                     )));
                 }

From 313f1b29523c8078a61accfc09e3897dd0300d4f Mon Sep 17 00:00:00 2001
From: Kevin Reid <kpreid@switchb.org>
Date: Sun, 20 Aug 2023 19:57:46 -0700
Subject: [PATCH 5/6] Test `create_surface_from_canvas()` error.

---
 tests/src/lib.rs                    | 48 ++++++++++++++---------------
 tests/tests/create_surface_error.rs | 27 ++++++++++++++++
 tests/tests/root.rs                 |  1 +
 3 files changed, 52 insertions(+), 24 deletions(-)
 create mode 100644 tests/tests/create_surface_error.rs

diff --git a/tests/src/lib.rs b/tests/src/lib.rs
index fb57d2a5a8..1d741f1812 100644
--- a/tests/src/lib.rs
+++ b/tests/src/lib.rs
@@ -312,7 +312,7 @@ pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(Te
         if #[cfg(any(not(target_arch = "wasm32"), target_os = "emscripten"))] {
             let canary_set = wgpu::hal::VALIDATION_CANARY.get_and_reset();
         } else {
-            let canary_set = _surface_guard.check_for_unreported_errors();
+            let canary_set = _surface_guard.unwrap().check_for_unreported_errors();
         }
     );
 
@@ -345,24 +345,18 @@ pub fn initialize_test(parameters: TestParameters, test_function: impl FnOnce(Te
     }
 }
 
-fn initialize_adapter() -> (Adapter, SurfaceGuard) {
-    let backends = wgpu::util::backend_bits_from_env().unwrap_or_else(Backends::all);
-    let dx12_shader_compiler = wgpu::util::dx12_shader_compiler_from_env().unwrap_or_default();
-    let gles_minor_version = wgpu::util::gles_minor_version_from_env().unwrap_or_default();
-    let instance = Instance::new(wgpu::InstanceDescriptor {
-        backends,
-        dx12_shader_compiler,
-        gles_minor_version,
-    });
-    let surface_guard;
+fn initialize_adapter() -> (Adapter, Option<SurfaceGuard>) {
+    let instance = initialize_instance();
+    let surface_guard: Option<SurfaceGuard>;
     let compatible_surface;
 
+    // Create a canvas iff we need a WebGL2RenderingContext to have a working device.
     #[cfg(not(all(
         target_arch = "wasm32",
         any(target_os = "emscripten", feature = "webgl")
     )))]
     {
-        surface_guard = SurfaceGuard {};
+        surface_guard = None;
         compatible_surface = None;
     }
     #[cfg(all(
@@ -398,7 +392,7 @@ fn initialize_adapter() -> (Adapter, SurfaceGuard) {
                 .expect("could not create surface from canvas")
         };
 
-        surface_guard = SurfaceGuard { canvas };
+        surface_guard = Some(SurfaceGuard { canvas });
 
         compatible_surface = Some(surface);
     }
@@ -413,12 +407,21 @@ fn initialize_adapter() -> (Adapter, SurfaceGuard) {
     (adapter, surface_guard)
 }
 
-struct SurfaceGuard {
-    #[cfg(all(
-        target_arch = "wasm32",
-        any(target_os = "emscripten", feature = "webgl")
-    ))]
-    canvas: web_sys::HtmlCanvasElement,
+pub fn initialize_instance() -> Instance {
+    let backends = wgpu::util::backend_bits_from_env().unwrap_or_else(Backends::all);
+    let dx12_shader_compiler = wgpu::util::dx12_shader_compiler_from_env().unwrap_or_default();
+    let gles_minor_version = wgpu::util::gles_minor_version_from_env().unwrap_or_default();
+    Instance::new(wgpu::InstanceDescriptor {
+        backends,
+        dx12_shader_compiler,
+        gles_minor_version,
+    })
+}
+
+// Public because it is used by tests of interacting with canvas
+pub struct SurfaceGuard {
+    #[cfg(target_arch = "wasm32")]
+    pub canvas: web_sys::HtmlCanvasElement,
 }
 
 impl SurfaceGuard {
@@ -452,11 +455,8 @@ impl Drop for SurfaceGuard {
     }
 }
 
-#[cfg(all(
-    target_arch = "wasm32",
-    any(target_os = "emscripten", feature = "webgl")
-))]
-fn create_html_canvas() -> web_sys::HtmlCanvasElement {
+#[cfg(target_arch = "wasm32")]
+pub fn create_html_canvas() -> web_sys::HtmlCanvasElement {
     use wasm_bindgen::JsCast;
 
     web_sys::window()
diff --git a/tests/tests/create_surface_error.rs b/tests/tests/create_surface_error.rs
new file mode 100644
index 0000000000..becfb3aa0c
--- /dev/null
+++ b/tests/tests/create_surface_error.rs
@@ -0,0 +1,27 @@
+//! Test that `create_surface_*()` accurately reports those errors we can provoke.
+
+/// This test applies to those cfgs that have a `create_surface_from_canvas` method, which
+/// include WebGL and WebGPU, but *not* Emscripten GLES.
+#[cfg(all(target_arch = "wasm32", not(target_os = "emscripten")))]
+#[wasm_bindgen_test::wasm_bindgen_test]
+fn canvas_get_context_returned_null() {
+    // Not using initialize_test() because that goes straight to creating the canvas for us.
+    let instance = wgpu_test::initialize_instance();
+    // Create canvas and cleanup on drop
+    let canvas_g = wgpu_test::SurfaceGuard {
+        canvas: wgpu_test::create_html_canvas(),
+    };
+    // Using a context id that is not "webgl2" or "webgpu" will render the canvas unusable by wgpu.
+    canvas_g.canvas.get_context("2d").unwrap();
+
+    let error = instance
+        .create_surface_from_canvas(canvas_g.canvas.clone())
+        .unwrap_err();
+
+    assert!(
+        error
+            .to_string()
+            .contains("canvas.getContext() returned null"),
+        "{error}"
+    );
+}
diff --git a/tests/tests/root.rs b/tests/tests/root.rs
index 86139c9bd2..8f7d457ed1 100644
--- a/tests/tests/root.rs
+++ b/tests/tests/root.rs
@@ -9,6 +9,7 @@ mod buffer;
 mod buffer_copy;
 mod buffer_usages;
 mod clear_texture;
+mod create_surface_error;
 mod device;
 mod encoder;
 mod example_wgsl;

From eebd9b67d5b2638a121b9f5a9bd04a1744c016fe Mon Sep 17 00:00:00 2001
From: Kevin Reid <kpreid@switchb.org>
Date: Mon, 21 Aug 2023 20:02:40 -0700
Subject: [PATCH 6/6] Ignore false positive `redundant_clone` from Clippy

---
 tests/tests/create_surface_error.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/tests/create_surface_error.rs b/tests/tests/create_surface_error.rs
index becfb3aa0c..f8962697ce 100644
--- a/tests/tests/create_surface_error.rs
+++ b/tests/tests/create_surface_error.rs
@@ -14,6 +14,7 @@ fn canvas_get_context_returned_null() {
     // Using a context id that is not "webgl2" or "webgpu" will render the canvas unusable by wgpu.
     canvas_g.canvas.get_context("2d").unwrap();
 
+    #[allow(clippy::redundant_clone)] // false positive — can't and shouldn't move out.
     let error = instance
         .create_surface_from_canvas(canvas_g.canvas.clone())
         .unwrap_err();