search

jycouet / VSTSExtensions

MIT License
30 stars 11 forks source link

On renovate error => Task Succeeded #9

Closed jycouet closed 4 years ago

jycouet commented 6 years ago

How to catch an internal renovate error?

DavidParks8 commented 6 years ago

There is a third parameter to exec that can be used to fail the task on std error. https://github.com/Microsoft/vsts-task-lib/blob/master/node/docs/vsts-task-lib.md#toolrunnerIExecOptions

jycouet commented 6 years ago

Hummm, thx for the doc. I had a look... but could't make it working.

Could have a look please? :)

DavidParks8 commented 6 years ago

I'll take a look. It's possible that renovate doesn't write to stderr, in which case it would still pass. I'll have to look through renovate to see. If that is the case, then hopefully renovate sets a different return code when it errors. If it doesn't then we're going to have trouble implementing this.

DavidParks8 commented 6 years ago

Unless I'm missing something, it looks like renovate doesn't ever explicitly set a return code, meaning success status is returned by default. I looked here and saw no process.exit(1) call.

DavidParks8 commented 6 years ago

And here it looks like the default behavior is to only pipe to stdout. Changes will be needed in renovate for us to detect failures.

DavidParks8 commented 6 years ago

So @jycouet, how would you like to proceed with this? I could open an issue in renovate, but you are already a contributor and would probably require less discussion to implement than a "newcomer" like me.

jycouet commented 6 years ago

Hey @DavidParks8, You or me can open an issue over there, to start a exchange with renovate. Then to implement, I can have a look with your help :)

Just to make sure we have the same needs. In VSTS the build task can be:

And when we run renovate we would like to reflect renovate status into the build status. My "problem" is that sometime an Error in renovate would only mean a Partially Succeed and not a fail.

So, do we have other way to implement this? Create a log file and parse the log file? depending on the result we can raise different build status?

DavidParks8 commented 6 years ago

I think the first thing we need to do is open the issue in renovate. I would very much appreciate if you initiated that 😄. Then, we need to determine all scenarios which should be a warning, all which should be successes, and all that should fail a build. After that we can discuss renovate process return codes and what event corresponds to what code.

While it is possible to analyze log files and stdout text, I would recommend against it. Such an implementation would be super brittle. Any slight change to log messages could break the vsts extension. This issue is compounded by the fact that people don't usually consider log message changes as a breaking change so we would only detect issues with the extension through bug reports.

It would be much less brittle to make a contract with the return codes of the renovate cli.

jycouet commented 6 years ago

Hey,

Actually more I think about it, more I don't know what to do! The idea is that if you have an error / warning if will be written in PR. So not really in the build system.

In the build it would be only when there is a real crash.

So I would think that it's good like this...

DavidParks8 commented 6 years ago

That would work fine, except for the fact that renovate has a blind catch-all statement that doesn't allow errors to bubble up to the build. I think we can minimally make the renovate process return a non-zero exit code when that happens. This would cover cases where, for example, the PR update request fails.