woodpecker-ci / woodpecker

Woodpecker is a simple yet powerful CI/CD engine with great extensibility.
https://woodpecker-ci.org
Apache License 2.0
3.88k stars 345 forks source link

Let webhook pass on pipeline parsion error #3829

Closed 6543 closed 1 week ago

6543 commented 1 week ago

Instead of show in e.g. gitea a 500, we get an 200 response and the actual pipeline info

this helps to debug and easyer to integrate ...

codecov[bot] commented 1 week ago

Codecov Report

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

Project coverage is 26.50%. Comparing base (402938e) to head (2f0f9fe).

:exclamation: Current head 2f0f9fe differs from pull request most recent head e484bdd

Please upload reports for the commit e484bdd to get more accurate results.

Files Patch % Lines
server/pipeline/create.go 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3829 +/- ## ========================================== - Coverage 28.06% 26.50% -1.57% ========================================== Files 363 367 +4 Lines 25108 27112 +2004 ========================================== + Hits 7047 7185 +138 - Misses 17536 19336 +1800 - Partials 525 591 +66 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

qwerty287 commented 1 week ago

I don't get how this can change something.

I checked all usages of Create and they just check err != nil and if there's an error, the returned pipeline itself is not used.

Besides that, I don't think it should return a 200 to the webhook. Yes, 500 is bad too, but something like 400?

6543 commented 1 week ago

juts test it ... update a pipeline definition to an invalid format ... e.g. non valid json

Before you got an 500 ... now you get 200

An other status is fine by me too but not 500

woodpecker-bot commented 1 week ago

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