feat: add rollback_on_failure feature #88
@@ -6,6 +6,7 @@ This repository is maintained by Gemini.
|
|||||||
|
|
||||||
* **Test-Driven Development (TDD):** Wherever possible, Test-Driven Development principles should be followed. Write tests before writing the code they are intended to validate.
|
* **Test-Driven Development (TDD):** Wherever possible, Test-Driven Development principles should be followed. Write tests before writing the code they are intended to validate.
|
||||||
* **Pre-commit Hooks:** Ensure that `pre-commit` hooks are installed and active before making any commits. This can be done by running `pre-commit install` in your local repository.
|
* **Pre-commit Hooks:** Ensure that `pre-commit` hooks are installed and active before making any commits. This can be done by running `pre-commit install` in your local repository.
|
||||||
|
* **Note for Gemini:** Git commits trigger pre-commit hooks, which can take several seconds (or minutes) to complete. Checking the command status for git commit is only appropriate every 120s.
|
||||||
|
|
||||||
## Repository Comparison
|
## Repository Comparison
|
||||||
|
|
||||||
|
|||||||
@@ -67,6 +67,10 @@ inputs:
|
|||||||
description: 'Print a progress update every N ticks when log_to_file=true (default: 10).'
|
description: 'Print a progress update every N ticks when log_to_file=true (default: 10).'
|
||||||
required: false
|
required: false
|
||||||
default: '10'
|
default: '10'
|
||||||
|
rollback_on_failure:
|
||||||
|
description: 'Automatically rollback to previous code if the monitor detects failures. Requires monitor > 0. (default: false)'
|
||||||
|
required: false
|
||||||
|
default: 'false'
|
||||||
outputs:
|
outputs:
|
||||||
saw_traceback:
|
saw_traceback:
|
||||||
description: true if a JS traceback was detected during monitoring.
|
description: true if a JS traceback was detected during monitoring.
|
||||||
|
|||||||
@@ -179,22 +179,55 @@ export async function postCode() {
|
|||||||
|
Philipp marked this conversation as resolved
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
let api = new ScreepsAPI(login_arguments);
|
let api = new ScreepsAPI(login_arguments);
|
||||||
if (token) {
|
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
if (!token) {
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
core.info(`Logging in as user ${username}`);
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
try {
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
await api.auth(username, password, login_arguments);
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
} catch (err) {
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
core.error(`Authentication error: ${err}`);
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
throw err;
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
}
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
}
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
let oldCode = null;
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
let rollbackOnFailure = false;
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
try {
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
rollbackOnFailure = core.getBooleanInput("rollback_on_failure");
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
} catch (e) {
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
// getBooleanInput throws if not 'true' or 'false', ignore
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
}
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
if (rollbackOnFailure) {
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
core.info(
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
`Downloading existing code from branch ${branch} for potential rollback...`,
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
);
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
try {
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
const getResponse = await api.code.get(branch);
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
if (getResponse && getResponse.ok && getResponse.modules) {
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
oldCode = getResponse.modules;
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
core.info(
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
`Successfully downloaded existing code (modules: ${Object.keys(oldCode).join(", ")})`,
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
);
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
} else {
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
core.warning(
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
AutoReview
commented
If the user explicitly requested If the user explicitly requested `rollback_on_failure`, and the download fails, should the action continue? Currently it results in a warning. If the deployment is critical, failing here might be safer.
|
|||||||
|
`Failed to download existing code, rollback will not be possible.`,
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
);
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
}
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
} catch (err) {
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
core.warning(
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
`Error downloading existing code: ${err.message}. Rollback will not be possible.`,
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
);
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
}
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
}
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
try {
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
const response = await api.code.set(branch, files_to_push);
|
const response = await api.code.set(branch, files_to_push);
|
||||||
core.info(JSON.stringify(response, null, 2));
|
core.info(JSON.stringify(response, null, 2));
|
||||||
core.info(`Code set successfully to ${branch}`);
|
core.info(`Code set successfully to ${branch}`);
|
||||||
} else {
|
} catch (err) {
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
core.info(`Logging in as user ${username}`);
|
core.error(`Upload error: ${err}`);
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
await Promise.resolve()
|
throw err;
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
.then(() => api.auth(username, password, login_arguments))
|
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
.then(() => api.code.set(branch, files_to_push))
|
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
.then(() => {
|
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
core.info(`Code set successfully to ${branch}`);
|
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
})
|
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
.catch((err) => {
|
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
core.error(`Upload error: ${err}`);
|
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
throw err;
|
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
});
|
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Console monitoring (optional)
|
// Console monitoring (optional)
|
||||||
@@ -215,6 +248,28 @@ export async function postCode() {
|
|||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
core.setOutput("saw_error_log", String(result.sawErrorLog));
|
core.setOutput("saw_error_log", String(result.sawErrorLog));
|
||||||
core.setOutput("saw_warning_log", String(result.sawWarningLog));
|
core.setOutput("saw_warning_log", String(result.sawWarningLog));
|
||||||
|
|
||||||
|
let shouldFail = false;
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
AutoReview
commented
This block duplicates the logic found inside This block duplicates the logic found inside `applyOnAction`. While necessary to determine if a rollback should occur before the final `setFailed` call, it might be worth refactoring to avoid inconsistent behavior if more failure conditions are added in the future.
|
|||||||
|
if (core.getInput("on_traceback") === "fail" && result.sawTraceback)
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
shouldFail = true;
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
if (core.getInput("on_error_log") === "fail" && result.sawErrorLog)
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
shouldFail = true;
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
if (core.getInput("on_warning_log") === "fail" && result.sawWarningLog)
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
AutoReview
commented
Consider defining these defaults in a central place or ensuring they are strictly pulled from Consider defining these defaults in a central place or ensuring they are strictly pulled from `action.yaml`. If `action.yaml` defaults change but these fallbacks don't, the behavior might become inconsistent.
|
|||||||
|
shouldFail = true;
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
if (shouldFail && rollbackOnFailure && oldCode) {
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
core.info(
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
"Action failed based on monitor configuration. Rolling back to previous code...",
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
);
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
try {
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
await api.code.set(branch, oldCode);
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
core.info(
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
`Successfully rolled back to previous code on branch ${branch}.`,
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
);
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
} catch (err) {
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
core.error(`Rollback failed: ${err}`);
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
}
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
}
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
|
|
||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
applyOnAction(
|
applyOnAction(
|
||||||
core.getInput("on_traceback"),
|
core.getInput("on_traceback"),
|
||||||
result.sawTraceback,
|
result.sawTraceback,
|
||||||
|
|||||||
|
Philipp marked this conversation as resolved
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
AutoReview
commented
The The `api.auth` call is now explicitly awaited outside the conditional block. This is cleaner than the previous `.then()` chain.
|
|||||||
The
api.authcall is now explicitly awaited outside the conditional block. This is cleaner than the previous.then()chain.The
api.authcall is now explicitly awaited outside the conditional block. This is cleaner than the previous.then()chain.