From 3b86c8a9339a295ec2af4afcfd43435d6c3607a8 Mon Sep 17 00:00:00 2001 From: Tristan Labelle Date: Wed, 3 Apr 2024 15:12:16 -0400 Subject: [PATCH 1/4] Preserve the IRestrictedErrorInfo after reading it. --- swiftwinrt/Resources/Support/Error.swift | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/swiftwinrt/Resources/Support/Error.swift b/swiftwinrt/Resources/Support/Error.swift index 4ae678b0..5b7ae8b6 100644 --- a/swiftwinrt/Resources/Support/Error.swift +++ b/swiftwinrt/Resources/Support/Error.swift @@ -104,8 +104,14 @@ public var E_XAMLPARSEFAILED : WinSDK.HRESULT { private func getErrorDescription(expecting hr: HRESULT) -> String? { var errorInfo: UnsafeMutablePointer? guard GetRestrictedErrorInfo(&errorInfo) == S_OK else { return nil } + guard let errorInfo else { return nil } + + // Getting the error info clears it, but it's valuable to keep it + // for crash diagnostics purposes. + SetRestrictedErrorInfo(errorInfo) + defer { - _ = errorInfo?.pointee.lpVtbl.pointee.Release(errorInfo) + _ = errorInfo.pointee.lpVtbl.pointee.Release(errorInfo) } var errorDescription: BSTR? @@ -117,7 +123,7 @@ private func getErrorDescription(expecting hr: HRESULT) -> String? { SysFreeString(capabilitySid) } var resultLocal: HRESULT = S_OK - _ = errorInfo?.pointee.lpVtbl.pointee.GetErrorDetails( + _ = errorInfo.pointee.lpVtbl.pointee.GetErrorDetails( errorInfo, &errorDescription, &resultLocal, From 393cda8eb8341fdd4cbc3025b92e4ddc8852411c Mon Sep 17 00:00:00 2001 From: Tristan Labelle Date: Wed, 3 Apr 2024 15:14:48 -0400 Subject: [PATCH 2/4] Better comments --- swiftwinrt/Resources/Support/Error.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/swiftwinrt/Resources/Support/Error.swift b/swiftwinrt/Resources/Support/Error.swift index 5b7ae8b6..61576649 100644 --- a/swiftwinrt/Resources/Support/Error.swift +++ b/swiftwinrt/Resources/Support/Error.swift @@ -106,8 +106,9 @@ private func getErrorDescription(expecting hr: HRESULT) -> String? { guard GetRestrictedErrorInfo(&errorInfo) == S_OK else { return nil } guard let errorInfo else { return nil } - // Getting the error info clears it, but it's valuable to keep it - // for crash diagnostics purposes. + // From the docs: https://learn.microsoft.com/en-us/windows/win32/api/roerrorapi/nf-roerrorapi-getrestrictederrorinfo + // > GetRestrictedErrorInfo transfers ownership of the error object to the caller and clears the error state for the thread. + // For crash reporting purposes, it's useful to preserve the error info state. SetRestrictedErrorInfo(errorInfo) defer { From eb216439c66bb81639c76ad85f8ead9a994a6acd Mon Sep 17 00:00:00 2001 From: Tristan Labelle Date: Wed, 3 Apr 2024 15:50:50 -0400 Subject: [PATCH 3/4] Switch to RoGetMatchingRestrictedErrorInfo --- swiftwinrt/Resources/Support/Error.swift | 44 +++++++++++++++--------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/swiftwinrt/Resources/Support/Error.swift b/swiftwinrt/Resources/Support/Error.swift index 61576649..be9c2530 100644 --- a/swiftwinrt/Resources/Support/Error.swift +++ b/swiftwinrt/Resources/Support/Error.swift @@ -101,20 +101,20 @@ public var E_XAMLPARSEFAILED : WinSDK.HRESULT { HRESULT(bitPattern: 0x802B000A) } -private func getErrorDescription(expecting hr: HRESULT) -> String? { +// Gets the restricted error info on the current thread for a given hresult, or creates a new one if it doesn't exist. +private func getOrCreateRestrictedErrorInfo(expecting hr: HRESULT) -> UnsafeMutablePointer? { + // Ensure that the thread has a restricted error info object var errorInfo: UnsafeMutablePointer? - guard GetRestrictedErrorInfo(&errorInfo) == S_OK else { return nil } - guard let errorInfo else { return nil } + guard RoGetMatchingRestrictedErrorInfo(hr, &errorInfo) == S_OK, let errorInfo else { return nil } // From the docs: https://learn.microsoft.com/en-us/windows/win32/api/roerrorapi/nf-roerrorapi-getrestrictederrorinfo // > GetRestrictedErrorInfo transfers ownership of the error object to the caller and clears the error state for the thread. // For crash reporting purposes, it's useful to preserve the error info state. SetRestrictedErrorInfo(errorInfo) + return errorInfo +} - defer { - _ = errorInfo.pointee.lpVtbl.pointee.Release(errorInfo) - } - +private func getErrorDescription(restrictedErrorInfo: UnsafeMutablePointer) -> String? { var errorDescription: BSTR? var restrictedDescription: BSTR? var capabilitySid: BSTR? @@ -123,16 +123,14 @@ private func getErrorDescription(expecting hr: HRESULT) -> String? { SysFreeString(restrictedDescription) SysFreeString(capabilitySid) } - var resultLocal: HRESULT = S_OK - _ = errorInfo.pointee.lpVtbl.pointee.GetErrorDetails( - errorInfo, + var hr: HRESULT = S_OK + guard restrictedErrorInfo.pointee.lpVtbl.pointee.GetErrorDetails( + restrictedErrorInfo, &errorDescription, - &resultLocal, + &hr, &restrictedDescription, - &capabilitySid) + &capabilitySid) == S_OK else { return nil } - guard resultLocal == hr else { return nil } - // Favor restrictedDescription as this is a more user friendly message, which // is intended to be displayed to the caller to help them understand why the // api call failed. If it's not set, then fallback to the generic error message @@ -142,7 +140,7 @@ private func getErrorDescription(expecting hr: HRESULT) -> String? { } else if SysStringLen(errorDescription) > 0 { return String(decodingCString: errorDescription!, as: UTF16.self) } else { - return nil + return hrToString(hr) } } @@ -171,7 +169,21 @@ public struct Error : Swift.Error, CustomStringConvertible { public let hr: HRESULT public init(hr: HRESULT) { - self.description = getErrorDescription(expecting: hr) ?? hrToString(hr) + // RoGetMatchingRestrictedErrorInfo creates an IRestrictedErrorInfo with a generic message if it can't find one for the given HRESULT. + var restrictedErrorInfo: UnsafeMutablePointer? + if RoGetMatchingRestrictedErrorInfo(hr, &restrictedErrorInfo) == S_OK, let restrictedErrorInfo { + defer { _ = restrictedErrorInfo.pointee.lpVtbl.pointee.Release(restrictedErrorInfo) } + + // From the docs: https://learn.microsoft.com/en-us/windows/win32/api/roerrorapi/nf-roerrorapi-getrestrictederrorinfo + // > GetRestrictedErrorInfo transfers ownership of the error object to the caller and clears the error state for the thread. + // Assume that RoGetMatchingRestrictedErrorInfo also clears the error state, + // but for crash reporting purposes, it's useful to preserve the it. + _ = SetRestrictedErrorInfo(restrictedErrorInfo) + + self.description = getErrorDescription(restrictedErrorInfo: restrictedErrorInfo) ?? hrToString(hr) + } else { + self.description = hrToString(hr) + } self.hr = hr } } From 9ebf088dc1d66996db3eb73ed45893332ca4d1e6 Mon Sep 17 00:00:00 2001 From: Tristan Labelle Date: Wed, 3 Apr 2024 17:18:58 -0400 Subject: [PATCH 4/4] Added a test --- swiftwinrt/Resources/Support/Error.swift | 13 --------- tests/test_app/main.swift | 34 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/swiftwinrt/Resources/Support/Error.swift b/swiftwinrt/Resources/Support/Error.swift index be9c2530..bc292545 100644 --- a/swiftwinrt/Resources/Support/Error.swift +++ b/swiftwinrt/Resources/Support/Error.swift @@ -101,19 +101,6 @@ public var E_XAMLPARSEFAILED : WinSDK.HRESULT { HRESULT(bitPattern: 0x802B000A) } -// Gets the restricted error info on the current thread for a given hresult, or creates a new one if it doesn't exist. -private func getOrCreateRestrictedErrorInfo(expecting hr: HRESULT) -> UnsafeMutablePointer? { - // Ensure that the thread has a restricted error info object - var errorInfo: UnsafeMutablePointer? - guard RoGetMatchingRestrictedErrorInfo(hr, &errorInfo) == S_OK, let errorInfo else { return nil } - - // From the docs: https://learn.microsoft.com/en-us/windows/win32/api/roerrorapi/nf-roerrorapi-getrestrictederrorinfo - // > GetRestrictedErrorInfo transfers ownership of the error object to the caller and clears the error state for the thread. - // For crash reporting purposes, it's useful to preserve the error info state. - SetRestrictedErrorInfo(errorInfo) - return errorInfo -} - private func getErrorDescription(restrictedErrorInfo: UnsafeMutablePointer) -> String? { var errorDescription: BSTR? var restrictedDescription: BSTR? diff --git a/tests/test_app/main.swift b/tests/test_app/main.swift index fb37caa7..c8c1366c 100644 --- a/tests/test_app/main.swift +++ b/tests/test_app/main.swift @@ -433,6 +433,39 @@ class SwiftWinRTTests : XCTestCase { } } + public func testIRestrictedErrorInfo() { + let message = "You are doing a bad thing" + do { + let classy = Class() + try classy.fail(message) + } catch { + var restrictedErrorInfo: UnsafeMutablePointer? + guard GetRestrictedErrorInfo(&restrictedErrorInfo) == S_OK, let restrictedErrorInfo else { + XCTFail("Failed to get error info") + return + } + defer { _ = restrictedErrorInfo.pointee.lpVtbl.pointee.Release(restrictedErrorInfo) } + + var errorDescription: BSTR? + var hr: HRESULT = S_OK + var restrictedDescription: BSTR? + var capabilitySid: BSTR? + defer { + SysFreeString(errorDescription) + SysFreeString(restrictedDescription) + SysFreeString(capabilitySid) + } + guard restrictedErrorInfo.pointee.lpVtbl.pointee.GetErrorDetails( + restrictedErrorInfo, &errorDescription, &hr, &restrictedDescription, &capabilitySid) == S_OK, + let restrictedDescription else { + XCTFail("Failed to get error description") + return + } + + XCTAssertEqual(String(decodingCString: restrictedDescription, as: UTF16.self), message) + } + } + public func testNoExcept() throws { let classy = Class() classy.noexceptVoid() @@ -459,6 +492,7 @@ var tests: [XCTestCaseEntry] = [ ("testStructWithIReference", SwiftWinRTTests.testStructWithIReference), ("testUnicode", SwiftWinRTTests.testUnicode), ("testErrorInfo", SwiftWinRTTests.testErrorInfo), + ("testIRestrictedErrorInfo", SwiftWinRTTests.testIRestrictedErrorInfo), ]) ] + valueBoxingTests + eventTests + collectionTests + aggregationTests + asyncTests + memoryManagementTests + bufferTests + weakReferenceTests