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

Improve ErrorPages examples #10209

Merged
merged 3 commits into from Nov 17, 2023

Conversation

arendhummeling
Copy link
Contributor

What does this PR do?

This PR attempts to describe a more comprehensive set of provider specific examples for the errors middleware configuration.

Motivation

While trying to implement the errors middleware, I ran into issues understanding the documentation. Using the Dynamic File Provider, I did not realise that writing the following wouldn't work:

http:
  middlewares:
    error-pages:
      errors:
        status:
         - "502,504"
        service: error-pages
        query: "/{status}.html"
  services:
    error-pages:
      loadBalancer:
        - url: {{ env "ERROR_PAGES_INTERNAL_URL" }}

The following line started appearing in my logs:

ERR error="strconv.Atoi: parsing "502,504": invalid syntax"

After some digging through the code, I found that the comma splitting did not occur in the same place as the dash splitting. This lead me to conclude it is likely provider specific syntax. I'm hoping to avoid other people following in my confusion with these changes.

More

  • Added/updated tests
  • Added/updated documentation

Additional Notes

This would be my first contribution to Traefik, so if there's anything I need to take care of before this PR can be reviewed or merge, please let me know and I will take care of it ASAP, thanks!
Should this PR be good to go, would it be helpful if I opened another PR to the 3.0 branch to update the documentation for v3 as well? Seeing as this specific page is virtually identical for v2 and v3.

docs/content/middlewares/http/errorpages.md Outdated Show resolved Hide resolved
Copy link
Contributor

@geraldcroes geraldcroes left a comment

Choose a reason for hiding this comment

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

Definitely a nice addition to the documentation, thank you for the examples!

For porting this to v3, unless I'm mistaken, we'll handle it as part of our existing process

Copy link
Member

@rtribotte rtribotte left a comment

Choose a reason for hiding this comment

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

I think it would be preferable to align also this note with the examples above.

docs/content/middlewares/http/errorpages.md Outdated Show resolved Hide resolved
docs/content/middlewares/http/errorpages.md Outdated Show resolved Hide resolved
@arendhummeling
Copy link
Contributor Author

I think it would be preferable to align also this note with the examples above.

Completely agreed, thanks

Copy link
Member

@rtribotte rtribotte left a comment

Choose a reason for hiding this comment

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

Thanks!

@arendhummeling arendhummeling force-pushed the docs/errorpages-example branch 2 times, most recently from 09bb75d to 3629e22 Compare November 15, 2023 21:19
arendhummeling and others added 3 commits November 17, 2023 10:37
While trying to implement the `errors` middleware, I ran into issues
understanding the documentation. Using the Dynamic File Provider,
I did not realise that writing the following wouldn't work:
```yaml
http:
  middlewares:
    error-pages:
      errors:
        status:
         - "502,504"
        service: error-pages
        query: "/{status}.html"
  services:
    error-pages:
      loadBalancer:
        - url: {{ env "ERROR_PAGES_INTERNAL_URL" }}
```
The following line started appearing in my logs:
 > ERR error="strconv.Atoi: parsing \"502,504\": invalid syntax"

After some digging through the code, I found that the comma splitting
did not occur in the same place as the dash splitting. This lead me
to conclude it is likely provider specific syntax.

Therefore it makes sense to me to provide more comprehensive provider
specific examples. I have tried to achieve this with this PR.

This would be my first contribution to Traefik, so if there's anything
I need to take care of before this PR can be reviewed or merge, please
let me know and I will take care of it ASAP, thanks!
Co-authored-by: Ludovic Fernandez <ldez@users.noreply.github.com>
@arendhummeling
Copy link
Contributor Author

@ldez sorry for the ping, would you be able to give this a re-review?

@traefiker traefiker merged commit 088fe3c into traefik:v2.10 Nov 17, 2023
9 checks passed
@rtribotte rtribotte changed the title docs: improve errorpages examples to avoid confusion Improve ErrorPages examples Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants