diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a9a4f3..c47a74f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixes +- 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)) ### Features diff --git a/modules/Sentry/private/New-HttpTransport.ps1 b/modules/Sentry/private/New-HttpTransport.ps1 deleted file mode 100644 index f3060c0..0000000 --- a/modules/Sentry/private/New-HttpTransport.ps1 +++ /dev/null @@ -1,16 +0,0 @@ -# Wrapper to expose Sentry.Internal.SdkComposer::CreateHttpTransport() -function New-HttpTransport { - [OutputType([Sentry.Extensibility.ITransport])] - [CmdletBinding()] - param( - [Parameter(Mandatory)] - [Sentry.SentryOptions] $options - ) - - $assembly = [Sentry.SentrySdk].Assembly - $type = $assembly.GetType('Sentry.Internal.SdkComposer') - $composer = [Activator]::CreateInstance($type, @($options)) - - $method = $type.GetMethod('CreateHttpTransport', [System.Reflection.BindingFlags]::Instance + [System.Reflection.BindingFlags]::NonPublic + [System.Reflection.BindingFlags]::Public) - return $method.Invoke($composer, @()) -} diff --git a/modules/Sentry/private/SynchronousWorker.ps1 b/modules/Sentry/private/SynchronousWorker.ps1 index 9a0a54f..6423d69 100644 --- a/modules/Sentry/private/SynchronousWorker.ps1 +++ b/modules/Sentry/private/SynchronousWorker.ps1 @@ -1,18 +1,16 @@ -. "$privateDir/New-HttpTransport.ps1" - class SynchronousWorker : Sentry.Extensibility.IBackgroundWorker { hidden [Sentry.Extensibility.ITransport] $transport hidden [Sentry.SentryOptions] $options hidden $unfinishedTasks = [System.Collections.Generic.List[System.Threading.Tasks.Task]]::new() SynchronousWorker([Sentry.SentryOptions] $options) { - $this.options = $options - - # Start from either the transport given on options, or create a new HTTP transport. - $this.transport = $options.Transport; - if ($null -eq $this.transport) { - $this.transport = New-HttpTransport($options) + # No fallback: the SDK builds its own default worker and transport when BackgroundWorker is left unset. + if ($null -eq $options.Transport) { + throw 'SynchronousWorker requires options.Transport to be set.' } + + $this.options = $options + $this.transport = $options.Transport } [bool] EnqueueEnvelope([Sentry.Protocol.Envelopes.Envelope] $envelope) { diff --git a/tests/sdk-internals.tests.ps1 b/tests/sdk-internals.tests.ps1 new file mode 100644 index 0000000..ea9998d --- /dev/null +++ b/tests/sdk-internals.tests.ps1 @@ -0,0 +1,64 @@ +# Reflection lookups fail silently, so pin the SDK internals the module depends on. A dependency bump that moves +# one of these must fail here rather than at send time. + +BeforeAll { + . "$PSScriptRoot/utils.ps1" + . "$PSScriptRoot/../modules/Sentry/private/SynchronousTransport.ps1" + + $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 + + function Get-SentryInternalType([string] $name) { + return [Sentry.SentrySdk].Assembly.GetType($name) + } + + function Should-BeMethod($method, [string] $returnType, [string[]] $parameterTypes) { + $method | Should -Not -BeNullOrEmpty + $method.ReturnType.FullName | Should -Be $returnType + ($method.GetParameters() | ForEach-Object { $_.ParameterType.FullName }) | Should -Be $parameterTypes + } +} + +Describe 'Sentry SDK internals used by SynchronousTransport' { + It 'HttpTransportBase.ProcessEnvelope' { + $method = [Sentry.Http.HttpTransportBase].GetMethod('ProcessEnvelope', $instanceFlags) + Should-BeMethod $method 'Sentry.Protocol.Envelopes.Envelope' @('Sentry.Protocol.Envelopes.Envelope') + } + + It 'HttpTransportBase.CreateRequest' { + $method = [Sentry.Http.HttpTransportBase].GetMethod('CreateRequest', $instanceFlags) + Should-BeMethod $method 'System.Net.Http.HttpRequestMessage' @('Sentry.Protocol.Envelopes.Envelope') + } + + It 'HttpTransportBase.HandleResponse' { + $method = [Sentry.Http.HttpTransportBase].GetMethod('HandleResponse', $instanceFlags) + Should-BeMethod $method 'System.Void' @('System.Net.Http.HttpResponseMessage', 'Sentry.Protocol.Envelopes.Envelope') + } + + It 'Sentry.Internal.Http.EnvelopeHttpContent' { + Get-SentryInternalType 'Sentry.Internal.Http.EnvelopeHttpContent' | Should -Not -BeNullOrEmpty + } + + It 'EnvelopeHttpContent.SerializeToStream' { + $type = Get-SentryInternalType 'Sentry.Internal.Http.EnvelopeHttpContent' + $method = $type.GetMethod('SerializeToStream', $instanceFlags) + Should-BeMethod $method 'System.Void' @('System.IO.Stream', 'System.Net.TransportContext', 'System.Threading.CancellationToken') + } +} + +Describe 'Sentry SDK internals used by Get-CurrentOptions' { + It 'SentrySdk.CurrentOptions' { + $property = [Sentry.SentrySdk].GetProperty('CurrentOptions', $staticFlags) + $property | Should -Not -BeNullOrEmpty + $property.PropertyType.FullName | Should -Be 'Sentry.SentryOptions' + } +} + +Describe 'SynchronousTransport' { + It 'resolves every internal member it needs' { + # The constructor does all of the above lookups and throws on any that fail. + $options = [Sentry.SentryOptions]::new() + $options.Dsn = 'https://key@127.0.0.1/1' + { [SynchronousTransport]::new($options) } | Should -Not -Throw + } +} diff --git a/tests/synchronous-worker.tests.ps1 b/tests/synchronous-worker.tests.ps1 new file mode 100644 index 0000000..26d6410 --- /dev/null +++ b/tests/synchronous-worker.tests.ps1 @@ -0,0 +1,58 @@ +BeforeAll { + . "$PSScriptRoot/utils.ps1" + . "$PSScriptRoot/../modules/Sentry/private/SynchronousWorker.ps1" + . "$PSScriptRoot/../modules/Sentry/private/Get-CurrentOptions.ps1" + $global:SentryPowershellRethrowErrors = $true +} + +AfterAll { + $global:SentryPowershellRethrowErrors = $false +} + +Describe 'SynchronousWorker' { + It 'throws when options.Transport is not set' { + # Only reachable when the SynchronousTransport constructor threw. + $options = [Sentry.SentryOptions]::new() + $options.Transport | Should -Be $null + { [SynchronousWorker]::new($options) } | Should -Throw '*requires options.Transport*' + } + + It 'sends envelopes through the transport from options' { + $options = [Sentry.SentryOptions]::new() + $options.Dsn = 'https://key@127.0.0.1/1' + $options.Transport = [RecordingTransport]::new() + + $sut = [SynchronousWorker]::new($options) + $envelope = [Sentry.Protocol.Envelopes.Envelope]::FromEvent([Sentry.SentryEvent]::new(), $null, $null, $null) + $sut.EnqueueEnvelope($envelope) | Should -Be $true + + $options.Transport.envelopes.Count | Should -Be 1 + $sut.get_QueuedItems() | Should -Be 0 + } +} + +Describe 'Start-Sentry worker composition' { + AfterEach { + Stop-Sentry + } + + It 'wires a SynchronousWorker on top of a SynchronousTransport' { + Start-Sentry { $_.Dsn = 'https://key@127.0.0.1/1' } + + $options = Get-CurrentOptions + $options.Transport.GetType().Name | Should -Be 'SynchronousTransport' + $options.BackgroundWorker.GetType().Name | Should -Be 'SynchronousWorker' + } + + It 'keeps a transport supplied through options' { + $transport = [RecordingTransport]::new() + Start-Sentry { + $_.Dsn = 'https://key@127.0.0.1/1' + $_.Transport = $transport + } + + $options = Get-CurrentOptions + $options.Transport | Should -Be $transport + $options.BackgroundWorker.GetType().Name | Should -Be 'SynchronousWorker' + } +}