Empty but valid variables throw a PHP error

This specifically relates to smtpHost, however I guess it could apply in other cases.

If email sending is OFF, the PLATFORM_SMTP_HOST variable is empty. When trying to set this into Drupal using $config, the config reader treats this null value as an error and throws a NotValidPlatformException on line 567 of Config.php.

We already have $config->isValidPlatform(), so there shouldn't be any need to throw this error. Either there should be a way to check whether email sending is on, or the empty value should be returned.

My workaround is to catch the error, but this seems a big hacky.

The better approach is to isset() check it first. That should return false if the value is empty/missing.

The problem with returning empty values is you then need a reliable sentinel, and sometimes null or empty string or 0 or whatever may not be a correct "empty" because it means something itself. We also don't necessarily know which of those is safe for different values. Some are strings, some are numeric.

Right, except that NotValidPlatform is the wrong exception, just because email is turned off. It's still valid.

Would you accept a patch to return the value from a method call?

For the record...

// Swiftmailer try { $config['swiftmailer.transport']['smtp_host'] = $platformsh->smtpHost; } catch (NotValidPlatformException $e) { $config['swiftmailer.transport']['smtp_host'] = ''; }

isset($paltformsh->smtpHost) ? $platformsh->smtpHost : ''

That would be better. Not ideal, but that should work now.

isset works, but this is not what the documentation claims it should do:

 * @property-read string $smtpHost
 *   The hostname of the Platform.sh default SMTP server (an empty string if
 *   emails are disabled on the environment).

@Berdir can you elaborate?

Not sure what else to say? The quoted docs say it is an empty string when not set but it actually throws an exception.

ok, looks like the root issue is here:

config-reader-php/src/Config.php

Line 511 in 09982f2

return $this->environmentVariables[$checkName] ?? null;

If enable_smtp is set to false for the environment, the corresponding environmental variable PLATFORM_SMTP_HOST is not present in the environment which causes the config reader to return a null. However, from what I understand, if enable_smtp is disable the environmental variable should still be set, but contain an empty value, not be absent altogether. I'm going to need to ask our internal engineering team to see if this is a change.

I assume you cant change the actual behaviour of the environment variable, but you could either adjust the code to fall back to an empty string for this one variable at least or change the documentation to say that it must be checked with isset().