From 8fad5329e3df2a15b3d5aa33f1c6d7e7750ffd06 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 7 Sep 2026 09:11:55 +0200 Subject: [PATCH] fix: Remove broken SdkComposer fallback transport SynchronousWorker fell back to New-HttpTransport when options.Transport was null. That helper worked when written against sentry-dotnet 4.1.2, but SdkComposer dropped CreateHttpTransport in 5.1.1 and gained a second constructor parameter in 5.16.1. Against the bundled 6.5.0 it throws MissingMethodException: Constructor on type 'Sentry.Internal.SdkComposer' not found. It went unnoticed across five SDK bumps because the branch is only reached once the SynchronousTransport constructor has already thrown, and Start-Sentry swallows the result and lets the SDK use its own default worker. Drop the fallback rather than repair it. What it built is the SDK's own default transport, which SentrySdk.Init composes correctly by itself when BackgroundWorker is left unset, so reaching further into internals here buys nothing. Runtime behaviour is unchanged. Add tests for the null-transport path and pin the internal SDK members the module still reflects on, by signature rather than by name. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + modules/Sentry/private/New-HttpTransport.ps1 | 16 ----- modules/Sentry/private/SynchronousWorker.ps1 | 14 ++--- tests/sdk-internals.tests.ps1 | 64 ++++++++++++++++++++ tests/synchronous-worker.tests.ps1 | 58 ++++++++++++++++++ 5 files changed, 129 insertions(+), 24 deletions(-) delete mode 100644 modules/Sentry/private/New-HttpTransport.ps1 create mode 100644 tests/sdk-internals.tests.ps1 create mode 100644 tests/synchronous-worker.tests.ps1 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' + } +}