Skip to content

Fix: warn on wrong configuration#1424

Merged
dunglas merged 4 commits intomainfrom
fix/warn-on-wrong-configuration
Mar 10, 2025
Merged

Fix: warn on wrong configuration#1424
dunglas merged 4 commits intomainfrom
fix/warn-on-wrong-configuration

Conversation

@AlliBalliBaba
Copy link
Copy Markdown
Contributor

@AlliBalliBaba AlliBalliBaba commented Mar 8, 2025

There have been multiple issues like #1419, #1423 or #1356 (comment) where people have unknowingly wrongly configured FrankenPHP.

Ideally, it should error on wrong configuration. This PR makes it so there's at least a warning to not break current Caddy configurations.

It also allows allows this shorthand worker configuration:

ENV FRANKENPHP_CONFIG="worker ./public/index.php watch"

@AlliBalliBaba
Copy link
Copy Markdown
Contributor Author

Looks like this deprecation notice is failing the static build

Deprecation Notice: PHP < 8.4 is deprecated, please upgrade your PHP version

Which is weird since this actually builds on 8.4

Comment thread caddy/caddy.go Outdated

wc.Num = v
if d.NextArg() {
// TODO: this should error in a V2
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not making an error right now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can if you're fine with it 👍 . It will legitimately break a lot of existing Caddyfiles though

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Alright this will error now. I tried to make the error message descriptive

@dunglas dunglas merged commit 09b8219 into main Mar 10, 2025
@dunglas dunglas deleted the fix/warn-on-wrong-configuration branch March 10, 2025 07:43
@dunglas
Copy link
Copy Markdown
Member

dunglas commented Mar 10, 2025

Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants