diff --git a/CHANGELOG.md b/CHANGELOG.md index c47a74f..99fd041 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixes +- Warn instead of silently ignoring `InAppInclude` / `InAppExclude` when a sentry-dotnet bump moves the internal members they are read through ([#144](https://github.com/getsentry/sentry-powershell/pull/144)) - Remove the `SdkComposer` fallback transport in `SynchronousWorker`, which has thrown since the sentry-dotnet 5.1.1 bump. The SDK's own default worker and transport are used instead ([#143](https://github.com/getsentry/sentry-powershell/pull/143)) - Silence CS1701/CS1702 warnings emitted by `Add-Type` when importing the module on PowerShell hosts whose runtime `System.Runtime` version differs from the one `Sentry.dll` was compiled against ([#129](https://github.com/getsentry/sentry-powershell/pull/129)) diff --git a/modules/Sentry/private/StackTraceProcessor.ps1 b/modules/Sentry/private/StackTraceProcessor.ps1 index 73fedeb..08ffaf9 100644 --- a/modules/Sentry/private/StackTraceProcessor.ps1 +++ b/modules/Sentry/private/StackTraceProcessor.ps1 @@ -8,6 +8,8 @@ class StackTraceProcessor : SentryEventProcessor { hidden [hashtable] $pwshModules = @{} hidden [System.Collections.IEnumerable] $inAppInclude hidden [System.Collections.IEnumerable] $inAppExclude + # A hashtable rather than typed FieldInfo properties, for the reason given in SynchronousTransport. + hidden [hashtable] $stringOrRegexFields = @{} StackTraceProcessor([Sentry.SentryOptions] $options) { $this.logger = $options.DiagnosticLogger @@ -23,29 +25,43 @@ class StackTraceProcessor : SentryEventProcessor { $this.modulePaths = $env:PSModulePath -split ':' } - # The SentryOptions.InAppInclude / InAppExclude lists are internal; read them via reflection. - # Entries are Sentry.StringOrRegex (string prefix or compiled regex) per the .NET SDK. - $flags = [System.Reflection.BindingFlags]::NonPublic -bor [System.Reflection.BindingFlags]::Instance - $includeProp = [Sentry.SentryOptions].GetProperty('InAppInclude', $flags) - $excludeProp = [Sentry.SentryOptions].GetProperty('InAppExclude', $flags) - if ($null -ne $includeProp) { - $this.inAppInclude = $includeProp.GetValue($options) + # InAppInclude / InAppExclude and the StringOrRegex value fields are internal to sentry-dotnet. + try { + $this.inAppInclude = [StackTraceProcessor]::GetInternalMember([Sentry.SentryOptions], 'InAppInclude').GetValue($options) + $this.inAppExclude = [StackTraceProcessor]::GetInternalMember([Sentry.SentryOptions], 'InAppExclude').GetValue($options) + $this.stringOrRegexFields['_string'] = [StackTraceProcessor]::GetInternalMember([Sentry.StringOrRegex], '_string') + $this.stringOrRegexFields['_regex'] = [StackTraceProcessor]::GetInternalMember([Sentry.StringOrRegex], '_regex') + } catch { + Write-Warning "Ignoring InAppInclude / InAppExclude: $_" + if ($global:SentryPowershellRethrowErrors -eq $true) { + throw + } + $this.inAppInclude = $null + $this.inAppExclude = $null + } + } + + # Throws on a miss so an SDK bump that moves a member is reported instead of silently dropping the option. + hidden static [System.Reflection.MemberInfo] GetInternalMember([type] $type, [string] $name) { + $flags = [System.Reflection.BindingFlags]::Instance -bor [System.Reflection.BindingFlags]::NonPublic -bor [System.Reflection.BindingFlags]::Public + $member = $type.GetProperty($name, $flags) + if ($null -eq $member) { + $member = $type.GetField($name, $flags) } - if ($null -ne $excludeProp) { - $this.inAppExclude = $excludeProp.GetValue($options) + if ($null -eq $member) { + throw "Failed to find internal member '$name' on $type" } + return $member } - hidden static [bool] MatchesAny([System.Collections.IEnumerable] $patterns, [string] $module) { + hidden [bool] MatchesAny([System.Collections.IEnumerable] $patterns, [string] $module) { if ($null -eq $patterns -or [string]::IsNullOrEmpty($module)) { return $false } foreach ($item in $patterns) { - # StringOrRegex has private _string / _regex fields, exactly one set. - $type = $item.GetType() - $flags = [System.Reflection.BindingFlags]::NonPublic -bor [System.Reflection.BindingFlags]::Instance - $stringValue = $type.GetField('_string', $flags).GetValue($item) - $regexValue = $type.GetField('_regex', $flags).GetValue($item) + # Exactly one of the two is set. + $stringValue = $this.stringOrRegexFields['_string'].GetValue($item) + $regexValue = $this.stringOrRegexFields['_regex'].GetValue($item) if (-not [string]::IsNullOrEmpty($stringValue)) { # Prefix match, matching .NET SDK namespace semantics ("Foo" matches "Foo" and "Foo.Bar"). # Case-insensitive on both halves, consistent with sentry-dotnet and PS module name resolution. @@ -65,10 +81,10 @@ class StackTraceProcessor : SentryEventProcessor { # InAppExclude wins, then InAppInclude. Falls back to the PS default: user-script frames (no module) # are in-app; module frames are not. This default differs from sentry-dotnet because PS module # frames are almost always third-party. - if ([StackTraceProcessor]::MatchesAny($this.inAppExclude, $module)) { + if ($this.MatchesAny($this.inAppExclude, $module)) { return $false } - if ([StackTraceProcessor]::MatchesAny($this.inAppInclude, $module)) { + if ($this.MatchesAny($this.inAppInclude, $module)) { return $true } return [string]::IsNullOrEmpty($module) diff --git a/tests/sdk-internals.tests.ps1 b/tests/sdk-internals.tests.ps1 index ea9998d..b99f24a 100644 --- a/tests/sdk-internals.tests.ps1 +++ b/tests/sdk-internals.tests.ps1 @@ -4,6 +4,8 @@ BeforeAll { . "$PSScriptRoot/utils.ps1" . "$PSScriptRoot/../modules/Sentry/private/SynchronousTransport.ps1" + . "$PSScriptRoot/../modules/Sentry/private/StackTraceProcessor.ps1" + $global:SentryPowershellRethrowErrors = $true $instanceFlags = [System.Reflection.BindingFlags]::Instance + [System.Reflection.BindingFlags]::NonPublic + [System.Reflection.BindingFlags]::Public $staticFlags = [System.Reflection.BindingFlags]::Static + [System.Reflection.BindingFlags]::NonPublic + [System.Reflection.BindingFlags]::Public @@ -19,6 +21,10 @@ BeforeAll { } } +AfterAll { + $global:SentryPowershellRethrowErrors = $false +} + Describe 'Sentry SDK internals used by SynchronousTransport' { It 'HttpTransportBase.ProcessEnvelope' { $method = [Sentry.Http.HttpTransportBase].GetMethod('ProcessEnvelope', $instanceFlags) @@ -54,6 +60,32 @@ Describe 'Sentry SDK internals used by Get-CurrentOptions' { } } +Describe 'Sentry SDK internals used by StackTraceProcessor' { + It 'SentryOptions.InAppInclude' { + $property = [Sentry.SentryOptions].GetProperty('InAppInclude', $instanceFlags) + $property | Should -Not -BeNullOrEmpty + [System.Collections.Generic.IEnumerable[Sentry.StringOrRegex]].IsAssignableFrom($property.PropertyType) | Should -BeTrue + } + + It 'SentryOptions.InAppExclude' { + $property = [Sentry.SentryOptions].GetProperty('InAppExclude', $instanceFlags) + $property | Should -Not -BeNullOrEmpty + [System.Collections.Generic.IEnumerable[Sentry.StringOrRegex]].IsAssignableFrom($property.PropertyType) | Should -BeTrue + } + + It 'StringOrRegex._string' { + $field = [Sentry.StringOrRegex].GetField('_string', $instanceFlags) + $field | Should -Not -BeNullOrEmpty + $field.FieldType.FullName | Should -Be 'System.String' + } + + It 'StringOrRegex._regex' { + $field = [Sentry.StringOrRegex].GetField('_regex', $instanceFlags) + $field | Should -Not -BeNullOrEmpty + $field.FieldType.FullName | Should -Be 'System.Text.RegularExpressions.Regex' + } +} + Describe 'SynchronousTransport' { It 'resolves every internal member it needs' { # The constructor does all of the above lookups and throws on any that fail. @@ -62,3 +94,9 @@ Describe 'SynchronousTransport' { { [SynchronousTransport]::new($options) } | Should -Not -Throw } } + +Describe 'StackTraceProcessor' { + It 'resolves every internal member it needs' { + { [StackTraceProcessor]::new([Sentry.SentryOptions]::new()) } | Should -Not -Throw + } +} diff --git a/tests/stacktrace-processor.tests.ps1 b/tests/stacktrace-processor.tests.ps1 index 5be40d7..662012b 100644 --- a/tests/stacktrace-processor.tests.ps1 +++ b/tests/stacktrace-processor.tests.ps1 @@ -103,13 +103,8 @@ at , : line 3' -split "[`r`n]+" $sut.ResolveInApp((MakeFrame 'Foo')) | Should -BeFalse } - It 'Sentry.StringOrRegex still exposes the private fields we reflect on' { - # ResolveInApp reaches into the internal _string / _regex fields of Sentry.StringOrRegex. - # If a sentry-dotnet bump renames these, this fails so we catch it at upgrade time rather - # than silently no-op-ing InAppInclude/InAppExclude in production. - $flags = [System.Reflection.BindingFlags]::NonPublic -bor [System.Reflection.BindingFlags]::Instance - [Sentry.StringOrRegex].GetField('_string', $flags) | Should -Not -BeNullOrEmpty - [Sentry.StringOrRegex].GetField('_regex', $flags) | Should -Not -BeNullOrEmpty + It 'Names the missing member when the SDK internals it reflects on have moved' { + { [StackTraceProcessor]::GetInternalMember([Sentry.StringOrRegex], 'NoSuchMember') } | Should -Throw '*NoSuchMember*' } } }