Skip to content

feat!: Envelope.FromEvent, FromFeedback and FromTransaction now take the logger as their last parameter - #5669

Merged
jamescrosswell merged 1 commit into
version7from
feat/envelope-attachments-before-logger
Oct 5, 2026
Merged

jamescrosswell merged 1 commit into
version7from
feat/envelope-attachments-before-logger

Conversation

@jamescrosswell

@jamescrosswell jamescrosswell commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Changelog Entry

  • Envelope.FromEvent, Envelope.FromFeedback and Envelope.FromTransaction now take the IDiagnosticLogger as their last parameter, after attachments (and sessionUpdate). This only affects code that creates envelopes directly and passes these arguments positionally; those calls no longer compile, so reorder the arguments or pass them by name.
    // Before
    Envelope.FromEvent(@event, logger, attachments, sessionUpdate);
    Envelope.FromTransaction(transaction, logger, attachments);
    
    // After
    Envelope.FromEvent(@event, attachments, sessionUpdate, logger);
    Envelope.FromTransaction(transaction, attachments, logger);

Summary

Moves the IDiagnosticLogger parameter of the public Envelope factory methods to the end, so the parameters that affect what goes into the envelope come first:

- FromEvent(SentryEvent @event, IDiagnosticLogger? logger = null, IReadOnlyCollection<SentryAttachment>? attachments = null, SessionUpdate? sessionUpdate = null)
+ FromEvent(SentryEvent @event, IReadOnlyCollection<SentryAttachment>? attachments = null, SessionUpdate? sessionUpdate = null, IDiagnosticLogger? logger = null)

- FromFeedback(SentryEvent @event, IDiagnosticLogger? logger = null, IReadOnlyCollection<SentryAttachment>? attachments = null, SessionUpdate? sessionUpdate = null)
+ FromFeedback(SentryEvent @event, IReadOnlyCollection<SentryAttachment>? attachments = null, SessionUpdate? sessionUpdate = null, IDiagnosticLogger? logger = null)

- FromTransaction(SentryTransaction transaction, IDiagnosticLogger? logger = null, IReadOnlyCollection<SentryAttachment>? attachments = null)
+ FromTransaction(SentryTransaction transaction, IReadOnlyCollection<SentryAttachment>? attachments = null, IDiagnosticLogger? logger = null)

The issue only asks for attachments to come before logger. Putting logger last in all three methods, after sessionUpdate too, gives every factory the same shape.

A positional call written against the old order can't silently bind to the wrong parameter: the argument types no longer line up, so it fails to compile.

Notes

  • sentry-unity only calls Envelope.FromEvent(new SentryEvent()), so it is unaffected.
  • ApiApprovalTests.Run.Net4_8.verified.txt can't be regenerated on macOS, so it was edited by hand with the same line changes the other TFMs produced.

Closes #5379

🤖 Generated with Claude Code

…logger last

Move the IDiagnosticLogger parameter after attachments (and sessionUpdate)
so the parameters that shape the envelope come first, consistently across
all three public factory methods.

Closes #5379

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jamescrosswell

Copy link
Copy Markdown
Collaborator Author

@ric-oliv even though this technically touches public types and is a breaking change, in practice I doubt many SDK users are using these types (you can see there were no changes to any of our samples projects as a result of the change, for example).

@jamescrosswell
jamescrosswell marked this pull request as ready for review October 5, 2026 04:22
@github-actions github-actions Bot added the risk: medium PR risk score: medium label Oct 5, 2026
@ric-oliv

ric-oliv commented Oct 5, 2026

Copy link
Copy Markdown
Member

Looks good!

@jamescrosswell
jamescrosswell merged commit 1eed232 into version7 Oct 5, 2026
42 checks passed
@jamescrosswell
jamescrosswell deleted the feat/envelope-attachments-before-logger branch October 5, 2026 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: medium PR risk score: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants