Skip to content

feat!: make Scope.Environment non-nullable - #5587

Closed
jamescrosswell wants to merge 2 commits into
version7from
feat/scope-environment-non-nullable
Closed

jamescrosswell wants to merge 2 commits into
version7from
feat/scope-environment-non-nullable

Conversation

@jamescrosswell

@jamescrosswell jamescrosswell commented Sep 17, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Scope.Environment is now declared as string rather than string?.

#5365 made the setter revert null to SentryOptions.Environment, but the property could still return null: a scope whose environment was never set had a null backing field, and SentryOptions.Environment is itself nullable. To make the non-nullable declaration truthful:

  • The getter falls back to SettingLocator.GetEnvironment() (options → SENTRY_ENVIRONMENT → production/debug), the same value the Enricher already stamps on events.
  • The setter keeps accepting null ([AllowNull], so it still satisfies IEventLike.Environment) and resets to that resolved default.
  • Scope.Apply copies only an explicitly set environment between scopes. Otherwise the non-null getter would make ??= never copy onto a cloned scope.

Changelog Entry

Scope.Environment is now non-nullable (#5587)

Notes for review

  • Breaking change (public API signature), hence targeting version7.
  • Side effect: Apply to an event now writes the resolved default environment from the scope rather than leaving it for the Enricher. The value is identical.
  • The Populate_RouteData_SetToScope snapshot now includes Environment: production, since serializing a scope reads the resolved value.
  • Apply_Environment_Null asserted a target scope's environment stays null, which is no longer possible. It now asserts the options fallback.

Closes #5388

🤖 Generated with Claude Code

When no environment has been set on the scope, the getter now falls back
to the environment resolved from the options (options, SENTRY_ENVIRONMENT,
then the default), so it never returns null.

Closes #5388

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (version7@8d4f28b). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/Sentry/Scope.cs 85.71% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             version7    #5587   +/-   ##
===========================================
  Coverage            ?   74.77%           
===========================================
  Files               ?      515           
  Lines               ?    18908           
  Branches            ?     3691           
===========================================
  Hits                ?    14138           
  Misses              ?     3893           
  Partials            ?      877           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jamescrosswell jamescrosswell linked an issue Sep 17, 2026 that may be closed by this pull request
Comment thread src/Sentry/Scope.cs Outdated
…tation

The nullable setter now only exists on the IEventLike face of Scope, so
assigning null to the public property is a compile error. Both accessors
funnel into SetEnvironment, where null means "unset".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jamescrosswell
jamescrosswell marked this pull request as ready for review September 27, 2026 22:52
@github-actions github-actions Bot added the risk: medium PR risk score: medium label Sep 27, 2026
@jamescrosswell

Copy link
Copy Markdown
Collaborator Author

@ric-oliv tbh not sure we even really need this change. If it's too much effort, I'm OK to close both the PR and the open issue as won't do.

@jamescrosswell

Copy link
Copy Markdown
Collaborator Author

@bitsandfoxes would this cause any problems for you guys? Per my comment above to Ricardo, I'm not dead set on doing this if we don't want to.

@ric-oliv

ric-oliv commented Oct 1, 2026

Copy link
Copy Markdown
Member

@jamescrosswell
I guess is fair to bring this in... The getter can no longer return null, so the type should say so :)

unless @bitsandfoxes sees any reason not to.

@jamescrosswell

Copy link
Copy Markdown
Collaborator Author

Just need to decide what we're doing about #5197 (comment) as if would render these changes unnecessary...

@jamescrosswell

Copy link
Copy Markdown
Collaborator Author

Made redundant/inappropriate by #5197

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.

Make Scope.Environment non nullable (next major)

2 participants