Skip to content

[DI] CSV env var processor does not work like an ordinary array #40654

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
stollr opened this issue Mar 31, 2021 · 6 comments
Closed

[DI] CSV env var processor does not work like an ordinary array #40654

stollr opened this issue Mar 31, 2021 · 6 comments

Comments

@stollr
Copy link

stollr commented Mar 31, 2021

Symfony version(s) affected: 5.2.6

Description
I am trying to setup monolog with a symfony_mailer handler. I want to configure the to_email with an ENV variable and usage of the csv processor so that I can add multiple receivers.

How to reproduce
I have the monolog.yaml looking like that:

monolog:
    handlers:
        main:
            type: fingers_crossed
            action_level: warning
            handler: symfony_mailer
        symfony_mailer:
            type:         symfony_mailer
            from_email:   '%env(MAILER_FROM)%'
            to_email:     '%env(csv:MAILER_ERRORLOG_TO)%'
            subject:      'Error! %%message%%'
            formatter:    monolog.formatter.html
            content_type: text/html

And the .env.local looks like that:

MAILER_ERRORLOG_TO=john@example.com,chris@example.com

This results in an InvalidTypeException

Invalid type for path "monolog.handlers.symfony_mailer.to_email.0". Expected one of "bool", "int", "float", "string", but got "array".

When I replace the env var with a real array, for instance

to_email:     ['john@example.com', 'chris@example.com']

the error is resolved. But as you can imagine this is no real solution.

I already have tried to figure out, what the problem is, but without real result. In Symfony's config definition BaseNode there is a normalizer method, which seem to replace the env variable with a nested array [[]] and this results in the error described above.

If you need more details please tell me.

@ro0NL
Copy link
Contributor

ro0NL commented Mar 31, 2021

See #27683, got stuck in #29270 :)

@stollr
Copy link
Author

stollr commented Mar 31, 2021

You are right. I see the problem. Would it be complicated to implementiert a checker, telling the developers that this combination of config and env is currently not supported, until there is a better solution?

Because it took me some time to figure out were the problem comes from.

@ro0NL
Copy link
Contributor

ro0NL commented Mar 31, 2021

i think the error message is the best hint we can generically provide currently :/

actually i think the right way to fix it is to make ifString return false for envs, introducing a new ifEnv() while at it.

Im not sure what BC breaks it'll cause but conceptually it should do the trick. (i think "env placeholders" are special, and should not match "string values", even they technically are).

Perhaps give it a try :)

@stollr
Copy link
Author

stollr commented Apr 1, 2021

What I do not get is: why is an empty array used at placeholder for an env variable? wouldn't be a string better suited?

@ro0NL
Copy link
Contributor

ro0NL commented Apr 1, 2021

csv: tells us values will always be of type array:

the case where ifString() could return true, is in case the env is also type string at least.

@nicolas-grekas
Copy link
Member

Closing as duplicate of #40906

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

No branches or pull requests

4 participants