Increase log level when failing to fetch YAML

[fernandrone]

Increase log level when failing to create pipelines. There are situations when woodpecker server may fail to create a pipeline and will only log the error under "debug", making it hard to debug the issue unless the user has explicitly activated debug logs.

To reproduce this, set the server environment variable WOODPECKER_CONFIG_SERVICE_ENDPOINT to an invalid service, i.e.

WOODPECKER_CONFIG_SERVICE_ENDPOINT: "http://invalid/config"

And it will fail to create a pipeline, but no error logs will be displayed, only a debug log:

{"level":"debug","repo":"<redacted>","error":"failed to fetch config via http (0) Post \"http://ci-template-plugin/config\": dial tcp: lookup invalid on 198.18.0.1:53: no such host","time":"2024-09-13T14:06:02Z","caller":"/go/src/github.com/woodpecker-ci/woodpecker/server/pipeline/create.go:90","message":"error while fetching config '' in 'refs/heads/fernandrone-patch-1' with user: 'fernandrone'"}

[fernandrone]

I didn't open an issue given it's a trivial change. I couldn't find in past PRs if it was an explicit decision to leave logs as "debug" instead of "error" either.

[qwerty287]

I agree to @zc-devs and @fernandrone, you thumbed up the comment #4107 (comment), so I guess this can be closed?

[zc-devs]

this can be closed?

But I'm convinced in regards the first change.

log.Error().Str("repo", repo.FullName).Err(configFetchErr).Msgf("error while fetching config

Even message itself says, that it's error :)

Edit 1
Fernandrone's case is valid, error should be used.
Is there can be error while fetching config because of user's mistake/misconfiguration, not system issue? If yes, then this cases should be distinguished somehow and for system causes should be error level, for user mistakes - debug.

[qwerty287]

You're right, this case - the config is not found in the repo - is handled separately. These logs only trigger if something really went wrong

[fernandrone]

I'll adjust the log level according to the comment so we can merge it. Sorry for not getting back to this sooner but I've had a few hectic past weeks. I'm away from my computer this weekend but should get this done next week 👍🏻

…

On Sun, Oct 6, 2024, 9:26 AM qwerty287 ***@***.***> wrote: You're right, this case - the config is not found in the repo - is handled separately. These logs only trigger if something really went wrong — Reply to this email directly, view it on GitHub <#4107 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABXO2TWRMGEI4M7FOSD55EDZ2EUADAVCNFSM6AAAAABOFPO6T2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJVGQZDCNBWGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[qwerty287]

Updated the corresponding line, this should be ready now. @woodpecker-ci/maintainers please review

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 28.22%. Comparing base (45e3b48) to head (cabf8da).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
server/pipeline/create.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4107   +/-   ##
=======================================
  Coverage   28.22%   28.22%           
=======================================
  Files         385      385           
  Lines       27847    27847           
=======================================
  Hits         7860     7860           
  Misses      19296    19296           
  Partials      691      691           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[woodpecker-bot]

Tearing down https://woodpecker-ci-woodpecker-pr-4107.surge.sh