Skip to content

Add enhanced function for error message handling in http request for configuration fetching#5712

Merged
6543 merged 36 commits into
woodpecker-ci:mainfrom
LUKIEYF:feature/5292
Feb 25, 2026
Merged

Add enhanced function for error message handling in http request for configuration fetching#5712
6543 merged 36 commits into
woodpecker-ci:mainfrom
LUKIEYF:feature/5292

Conversation

@LUKIEYF
Copy link
Copy Markdown
Contributor

@LUKIEYF LUKIEYF commented Nov 3, 2025

closes #5292

  1. add enhanced function for error handler to log out the exact reason that the config is failed to fetch

  2. once the http status is not correct in certain status code and the old config data is available, use the old config data and log the message to warn the user.

  3. related enhancedHttpError() function testing codes and related createPipeline() function scenarios testings are added.

please kindly review and give the comments, thank you all! : )

@woodpecker-bot
Copy link
Copy Markdown
Contributor

woodpecker-bot commented Nov 3, 2025

Surge PR preview deployment was removed

@6543 6543 changed the title feature/5292: add enhanced function for error message handling in http request for configuration fetching. Add enhanced function for error message handling in http request for configuration fetching. Nov 3, 2025
@6543
Copy link
Copy Markdown
Member

6543 commented Nov 3, 2025

please dont include issue number in title

@qwerty287 qwerty287 changed the title Add enhanced function for error message handling in http request for configuration fetching. Add enhanced function for error message handling in http request for configuration fetching Nov 4, 2025
@qwerty287 qwerty287 added server enhancement improve existing features labels Nov 4, 2025
LUKIEYF and others added 4 commits November 5, 2025 11:53
I found that it has the restriction of import of the shared/ toward the woodpecker-go, therefore I add the codes into the woodpecker-go either.
…ithub.com:LUKIEYF/woodpecker into feature/5292
@qwerty287
Copy link
Copy Markdown
Contributor

Hi @LUKIEYF sorry for not coming back to you for a long time. Can you check the linter and the tests?

@LUKIEYF
Copy link
Copy Markdown
Contributor Author

LUKIEYF commented Nov 28, 2025

Hi @LUKIEYF sorry for not coming back to you for a long time. Can you check the linter and the tests?

No problem, buddy. Will do on Monday as soon as possible. :)

@LUKIEYF
Copy link
Copy Markdown
Contributor Author

LUKIEYF commented Dec 1, 2025

The linter and test case issues are solved, please review. thank you!

Copy link
Copy Markdown
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

the server/services/utils/http_error.go is quite extensive. I wonder if we could reuse it in other woodpecker code paths too ...

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 1, 2025

Codecov Report

❌ Patch coverage is 42.47788% with 65 lines in your changes missing coverage. Please review.
✅ Project coverage is 21.75%. Comparing base (87230ad) to head (757f58f).

Files with missing lines Patch % Lines
server/services/utils/http_error.go 44.21% 39 Missing and 14 partials ⚠️
server/pipeline/create.go 0.00% 8 Missing ⚠️
server/services/config/http.go 66.66% 2 Missing and 1 partial ⚠️
server/services/utils/http.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5712      +/-   ##
==========================================
+ Coverage   21.61%   21.75%   +0.14%     
==========================================
  Files         428      429       +1     
  Lines       38738    38844     +106     
==========================================
+ Hits         8372     8450      +78     
- Misses      29596    29604       +8     
- Partials      770      790      +20     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@6543 6543 requested a review from qwerty287 December 1, 2025 05:23
Comment thread server/services/config/http.go Outdated
Comment thread server/services/utils/http_error.go Outdated
Comment thread shared/httputil/http_error.go
Comment thread server/services/config/http.go Outdated
LUKIEYF and others added 5 commits December 2, 2025 10:27
Co-authored-by: qwerty287 <80460567+qwerty287@users.noreply.github.com>
Co-authored-by: qwerty287 <80460567+qwerty287@users.noreply.github.com>
@LUKIEYF
Copy link
Copy Markdown
Contributor Author

LUKIEYF commented Dec 3, 2025

the server/services/utils/http_error.go is quite extensive. I wonder if we could reuse it in other woodpecker code paths too ...

I have moved the http_error.go and http_error_test.go to the /shared/httputil, please see whether it is proper. thank you!

Comment thread server/services/config/http.go Outdated
Comment thread shared/httputil/http_error.go
6543 and others added 4 commits February 25, 2026 12:55
Co-authored-by: qwerty287 <80460567+qwerty287@users.noreply.github.com>
Co-authored-by: qwerty287 <80460567+qwerty287@users.noreply.github.com>
@6543
Copy link
Copy Markdown
Member

6543 commented Feb 25, 2026

The endpoint should be removed from all errors in this method as well and shouldn't be necessary in the method anymore.

it is just used to grap the host name

@6543 6543 merged commit b806e98 into woodpecker-ci:main Feb 25, 2026
7 of 8 checks passed
@woodpecker-bot woodpecker-bot mentioned this pull request Feb 25, 2026
1 task
@qwerty287
Copy link
Copy Markdown
Contributor

@6543 no. If there's an error, it is set to the full endpoint.

I still consider this a security risk, nothing critical, but I would not return that endpoint in the error. And it's also possible that the admin would like to hide the host as well.

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

Labels

enhancement improve existing features server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add detailed and clear error messages for config fetching

4 participants