Skip to content

Conversation

Litarnus
Copy link
Contributor

@Litarnus Litarnus commented Sep 17, 2025

Description

This PR replaces symfony/options-resolver with a custom option resolver that acts almost as a drop in replacement, but comes with a reduced feature set.
Notable changes between symfony and our resolver:

  • Invalid values will no longer throw exceptions. Instead, if will fall back to the default value and produce a log message.
  • The defaults will act as a schema, meaning that added values that have no default value will be ignored
  • Values will be validated before and after normalization. The reason is so that normalizers will not be called with invalid values, removing the need to validate within normalizers.
  • Defaults should be specified after declaring validation in order to get validated and normalized.

I also had to replace usages of PHP 8+ features since they used a polyfill which was pulled in through the symfony/options-resolver.

Issues

resolves #1896
closes PHP-24

Copy link

linear bot commented Sep 18, 2025

if (!$isValid) {
// Since defaults are used as fallback values if passed options are invalid, we want to
// get hard errors here to make sure we have something to fall back to.
throw new \InvalidArgumentException(\sprintf('Invalid default for option "%s"', $option));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the throw here to make it easier for us to debug and to make sure we have proper defaults that adhere to the specified rules.
As per comment, we also need some defaults to fall back to so I think it makes sense to keep this, but I'm open for discussions

@Litarnus Litarnus marked this pull request as ready for review September 18, 2025 11:44
@Litarnus Litarnus self-assigned this Sep 18, 2025
cursor[bot]

This comment was marked as outdated.

@Litarnus Litarnus added the 5.x label Sep 22, 2025
# Conflicts:
#	src/Event.php
#	src/EventHint.php
#	src/Integration/IntegrationRegistry.php
#	src/Options.php
#	src/Stacktrace.php
#	src/State/Scope.php
#	src/UserDataBag.php
cursor[bot]

This comment was marked as outdated.

$logger = $options['logger'] ?? $this->getLoggerOrNullLogger();

return $this->resolver->resolve($options, $logger);
}
Copy link

Choose a reason for hiding this comment

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

Bug: Logger Initialization Issue

When resolveWithLogger is called from the constructor, it attempts to retrieve a logger. If no logger is explicitly provided in the constructor options, the fallback path accesses $this->options['logger'] before $this->options has been initialized, causing a fatal error.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant