Skip to content
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

MAIL_DEFAULT_SENDER format causes a crash on Docker #1005

Closed
avalou opened this issue Mar 17, 2022 · 3 comments · Fixed by #1007
Closed

MAIL_DEFAULT_SENDER format causes a crash on Docker #1005

avalou opened this issue Mar 17, 2022 · 3 comments · Fixed by #1007

Comments

@avalou
Copy link

avalou commented Mar 17, 2022

Hello,

So I'm hosting ihm with docker, and a mariadb container for the db. If I try to create a new project, I get a 500 error and the following error stack on the server. I can anyway access to this project if I try to log into it.

ihatemoney_1  | ERROR [ihatemoney.run] Exception on /create [POST]
ihatemoney_1  | Traceback (most recent call last):
ihatemoney_1  |   File "/usr/local/lib/python3.10/site-packages/flask/app.py", line 2073, in wsgi_app
ihatemoney_1  |     response = self.full_dispatch_request()
ihatemoney_1  |   File "/usr/local/lib/python3.10/site-packages/flask/app.py", line 1518, in full_dispatch_request
ihatemoney_1  |     rv = self.handle_user_exception(e)
ihatemoney_1  |   File "/usr/local/lib/python3.10/site-packages/flask_restful/__init__.py", line 271, in error_router
ihatemoney_1  |     return original_handler(e)
ihatemoney_1  |   File "/usr/local/lib/python3.10/site-packages/flask/app.py", line 1516, in full_dispatch_request
ihatemoney_1  |     rv = self.dispatch_request()
ihatemoney_1  |   File "/usr/local/lib/python3.10/site-packages/flask/app.py", line 1502, in dispatch_request
ihatemoney_1  |     return self.ensure_sync(self.view_functions[rule.endpoint])(**req.view_args)
ihatemoney_1  |   File "/src/ihatemoney/web.py", line 92, in admin_auth
ihatemoney_1  |     return f(*args, **kws)
ihatemoney_1  |   File "/src/ihatemoney/web.py", line 333, in create_project
ihatemoney_1  |     flash_email_error(
ihatemoney_1  |   File "/src/ihatemoney/utils.py", line 56, in flash_email_error
ihatemoney_1  |     (admin_name, admin_email) = current_app.config.get("MAIL_DEFAULT_SENDER")
ihatemoney_1  | ValueError: too many values to unpack (expected 2)
@zorun
Copy link
Collaborator

zorun commented Mar 17, 2022

Right, this is because MAIL_DEFAULT_SENDER is set as a string in Docker, while it's supposed to be a tuple.

This bug has been here for a long time, but up to now the variable was only used by flask-mail, which supports either a tuple or a simple string, so there was no visible errors. But probably the string passed to flask-mail had an incorrect format anyway.

But now, with #965, we also use MAIL_DEFAULT_SENDER directly, which causes the error you see.

@almet @Glandos @youegraillot what do you think? It doesn't seem possible to pass tuples through Docker. I think we should just change this variable to be a simple string, with a format like email@address or Name <email@address>. And add a conversion from tuple to string for backwards compatibility.

@zorun zorun changed the title Error happening when creating a project [Docker] MAIL_DEFAULT_SENDER format causes a crash on Docker Mar 17, 2022
@avalou
Copy link
Author

avalou commented Mar 17, 2022

Is it related to the fact that I left default values to email related environment variables ? I do not plan to use email services.

      - MAIL_DEFAULT_SENDER=('Budget manager', 'admin@example.com')
      - MAIL_PASSWORD=
      - MAIL_PORT=25
      - MAIL_SERVER=localhost
      - MAIL_USE_SSL=False
      - MAIL_USE_TLS=False
      - MAIL_USERNAME=

@almet
Copy link
Member

almet commented Mar 18, 2022

I think we should just change this variable to be a simple string, with a format like email@address or Name email@address. And add a conversion from tuple to string for backwards compatibility.

I agree with this, it seems weird to have a tuple here anyway. Let's change this (and support backward compat of course) :-)

@Glandos Glandos self-assigned this Mar 20, 2022
Glandos added a commit to Glandos/ihatemoney that referenced this issue Mar 20, 2022
zorun pushed a commit to Glandos/ihatemoney that referenced this issue Apr 2, 2022
zorun pushed a commit that referenced this issue Apr 7, 2022
TomRoussel pushed a commit to TomRoussel/ihatemoney that referenced this issue Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants