Skip to content

fix: Remove broken SdkComposer fallback transport - #143

Open
vaind wants to merge 1 commit into
mainfrom
fix/remove-broken-fallback-transport
Open

fix: Remove broken SdkComposer fallback transport#143
vaind wants to merge 1 commit into
mainfrom
fix/remove-broken-fallback-transport

Conversation

@vaind

@vaind vaind commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Problem

SynchronousWorker fell back to New-HttpTransport when options.Transport was null. The helper reflected on Sentry.Internal.SdkComposer, and the SDK has since moved out from under it twice:

sentry-dotnet ctor args CreateHttpTransport Result
4.1.2 (when written, Mar 2024) 1 yes works
5.0.1 1 yes works
5.1.1 (Feb 2025) 1 no GetMethod returns $null, invoking it throws
5.16.1 (Oct 2025) 2 no now fails earlier, at construction
6.5.0 (bundled) 2 no MissingMethodException

Running the helper verbatim against the bundled 6.5.0 gives:

THREW: System.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. Start-Sentry then swallowed the exception, logged a warning, and the SDK carried on with its own default worker. No test covered the path.

Approach

Dropping the fallback rather than repairing it. What it built is the SDK's own default transport, which SentrySdk.Init composes correctly by itself when BackgroundWorker is left unset. Reaching further into internals — at the exact moment those internals have demonstrably moved — buys nothing. Runtime behaviour is unchanged; the failure is just reported clearly now.

Relationship to #38 / #138

This is independent of whether we keep the synchronous worker at all, which is the remaining work on #38 and is gated on what #138 shows on the Windows legs.

A broken fallback is not evidence that the synchronous worker is unnecessary. The fallback is only reachable once the SynchronousTransport constructor has thrown, and it doesn't throw: Windows CI (windows-2022 / windows-2025, Windows PowerShell 5.1 / net462) is green with SentryPowershellRethrowErrors = $true, which makes Start-Sentry rethrow instead of swallow. So the worker itself has been running as designed all along, on every platform — only the already-degraded path changed behaviour in Feb 2025.

Deleting the fallback is correct under either outcome of #38: if the worker stays, this is dead code rebuilding the SDK's own default; if we move to a genuinely asynchronous worker, it gets deleted along with the worker. In that second case most of tests/sdk-internals.tests.ps1 would go with SynchronousTransport too — it protects the interim, which may be a while.

Notes for reviewers

  • tests/synchronous-worker.tests.ps1 covers the null-transport path, which previously threw MissingMethodException.
  • tests/sdk-internals.tests.ps1 pins the remaining internal members by return and parameter types, not just by name. The 5.1.1 regression above is exactly the shape it catches: the lookup silently returned $null and nothing failed until send time. I checked the guards can fail by simulating drift on three of them.
  • SentrySdk.CurrentOptions was not in the original report; I found it while auditing, and it degrades the same silent way.

Full suite against 6.5.0: 106 passed, 3 skipped (pre-existing Windows-only tests).

🤖 Generated with Claude Code

Comment thread tests/sdk-internals.tests.ps1 Dismissed
Comment thread tests/sdk-internals.tests.ps1 Dismissed
Comment thread tests/sdk-internals.tests.ps1 Dismissed
Comment thread tests/synchronous-worker.tests.ps1 Dismissed
Comment thread tests/synchronous-worker.tests.ps1 Dismissed
Comment thread tests/sdk-internals.tests.ps1 Dismissed
Comment thread tests/sdk-internals.tests.ps1 Dismissed
Comment thread tests/sdk-internals.tests.ps1 Dismissed
Comment thread tests/sdk-internals.tests.ps1 Dismissed
@vaind
vaind force-pushed the fix/remove-broken-fallback-transport branch 2 times, most recently from 4bb2111 to 486460f Compare September 7, 2026 07:28
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 <noreply@anthropic.com>
@vaind
vaind force-pushed the fix/remove-broken-fallback-transport branch from 486460f to 8fad532 Compare September 7, 2026 07:41
@vaind
vaind requested a review from jamescrosswell September 7, 2026 07:44

@jamescrosswell jamescrosswell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Possibly as a follow up, a similar fix could be applied to other code that reflects on non-public members in sentry-dotnet:

$includeProp = [Sentry.SentryOptions].GetProperty('InAppInclude', $flags)
$excludeProp = [Sentry.SentryOptions].GetProperty('InAppExclude', $flags)
if ($null -ne $includeProp) {
$this.inAppInclude = $includeProp.GetValue($options)
}
if ($null -ne $excludeProp) {
$this.inAppExclude = $excludeProp.GetValue($options)
}
}

$stringValue = $type.GetField('_string', $flags).GetValue($item)
$regexValue = $type.GetField('_regex', $flags).GetValue($item)
if (-not [string]::IsNullOrEmpty($stringValue)) {
# Prefix match, matching .NET SDK namespace semantics ("Foo" matches "Foo" and "Foo.Bar").
if ($module -eq $stringValue -or $module.StartsWith("$stringValue.")) {
return $true
}
} elseif ($null -ne $regexValue -and $regexValue.IsMatch($module)) {
return $true

That last one saw some change just today (but nothing that would break PS luckily)!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants