Skip to content

Commit

Permalink
fix(github-actions): add logging and set undefined for repoSha and …
Browse files Browse the repository at this point in the history 
…`repoFileContent`

Adds logging to aide in diagnosing issues.

Additionally, when declaring the `repoSha` and `repoFileContent` values, set the default value explicitly as `undefined`.
When not initialized as `undefined`, the only set of the value is done without a `const` or `let` which allows the value
to escape the block it is defined in.  This results in the `sha` value in the API call to be incorrect because it uses a
previous files `sha` value, or the file content as well.
  • Loading branch information
@josephperrott
josephperrott committed Jun 30, 2022
1 parent 050669a commit 9db43e7
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 47 deletions.
97 changes: 54 additions & 43 deletions github-actions/org-file-sync/main.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -761,12 +761,12 @@ var require_lib = __commonJS({
throw new Error("Client has already been disposed.");
}
const parsedUrl = new URL(requestUrl);
let info2 = this._prepareRequest(verb, parsedUrl, headers);
let info3 = this._prepareRequest(verb, parsedUrl, headers);
const maxTries = this._allowRetries && RetryableHttpVerbs.includes(verb) ? this._maxRetries + 1 : 1;
let numTries = 0;
let response;
do {
response = yield this.requestRaw(info2, data);
response = yield this.requestRaw(info3, data);
if (response && response.message && response.message.statusCode === HttpCodes.Unauthorized) {
let authenticationHandler;
for (const handler of this.handlers) {
Expand All @@ -776,7 +776,7 @@ var require_lib = __commonJS({
}
}
if (authenticationHandler) {
return authenticationHandler.handleAuthentication(this, info2, data);
return authenticationHandler.handleAuthentication(this, info3, data);
} else {
return response;
}
Expand All @@ -799,8 +799,8 @@ var require_lib = __commonJS({
}
}
}
info2 = this._prepareRequest(verb, parsedRedirectUrl, headers);
response = yield this.requestRaw(info2, data);
info3 = this._prepareRequest(verb, parsedRedirectUrl, headers);
response = yield this.requestRaw(info3, data);
redirectsRemaining--;
}
if (!response.message.statusCode || !HttpResponseRetryCodes.includes(response.message.statusCode)) {
Expand All @@ -821,7 +821,7 @@ var require_lib = __commonJS({
}
this._disposed = true;
}
requestRaw(info2, data) {
requestRaw(info3, data) {
return __awaiter(this, void 0, void 0, function* () {
return new Promise((resolve, reject) => {
function callbackForResult(err, res) {
Expand All @@ -833,16 +833,16 @@ var require_lib = __commonJS({
resolve(res);
}
}
this.requestRawWithCallback(info2, data, callbackForResult);
this.requestRawWithCallback(info3, data, callbackForResult);
});
});
}
requestRawWithCallback(info2, data, onResult) {
requestRawWithCallback(info3, data, onResult) {
if (typeof data === "string") {
if (!info2.options.headers) {
info2.options.headers = {};
if (!info3.options.headers) {
info3.options.headers = {};
}
info2.options.headers["Content-Length"] = Buffer.byteLength(data, "utf8");
info3.options.headers["Content-Length"] = Buffer.byteLength(data, "utf8");
}
let callbackCalled = false;
function handleResult(err, res) {
Expand All @@ -851,7 +851,7 @@ var require_lib = __commonJS({
onResult(err, res);
}
}
const req = info2.httpModule.request(info2.options, (msg) => {
const req = info3.httpModule.request(info3.options, (msg) => {
const res = new HttpClientResponse(msg);
handleResult(void 0, res);
});
Expand All @@ -863,7 +863,7 @@ var require_lib = __commonJS({
if (socket) {
socket.end();
}
handleResult(new Error(`Request timeout: ${info2.options.path}`));
handleResult(new Error(`Request timeout: ${info3.options.path}`));
});
req.on("error", function(err) {
handleResult(err);
Expand All @@ -885,27 +885,27 @@ var require_lib = __commonJS({
return this._getAgent(parsedUrl);
}
_prepareRequest(method, requestUrl, headers) {
const info2 = {};
info2.parsedUrl = requestUrl;
const usingSsl = info2.parsedUrl.protocol === "https:";
info2.httpModule = usingSsl ? https : http;
const info3 = {};
info3.parsedUrl = requestUrl;
const usingSsl = info3.parsedUrl.protocol === "https:";
info3.httpModule = usingSsl ? https : http;
const defaultPort = usingSsl ? 443 : 80;
info2.options = {};
info2.options.host = info2.parsedUrl.hostname;
info2.options.port = info2.parsedUrl.port ? parseInt(info2.parsedUrl.port) : defaultPort;
info2.options.path = (info2.parsedUrl.pathname || "") + (info2.parsedUrl.search || "");
info2.options.method = method;
info2.options.headers = this._mergeHeaders(headers);
info3.options = {};
info3.options.host = info3.parsedUrl.hostname;
info3.options.port = info3.parsedUrl.port ? parseInt(info3.parsedUrl.port) : defaultPort;
info3.options.path = (info3.parsedUrl.pathname || "") + (info3.parsedUrl.search || "");
info3.options.method = method;
info3.options.headers = this._mergeHeaders(headers);
if (this.userAgent != null) {
info2.options.headers["user-agent"] = this.userAgent;
info3.options.headers["user-agent"] = this.userAgent;
}
info2.options.agent = this._getAgent(info2.parsedUrl);
info3.options.agent = this._getAgent(info3.parsedUrl);
if (this.handlers) {
for (const handler of this.handlers) {
handler.prepareRequest(info2.options);
handler.prepareRequest(info3.options);
}
}
return info2;
return info3;
}
_mergeHeaders(headers) {
if (this.requestOptions && this.requestOptions.headers) {
Expand Down Expand Up @@ -1551,39 +1551,39 @@ Support boolean input list: \`true | True | TRUE | false | False | FALSE\``);
command_1.issueCommand("error", utils_1.toCommandProperties(properties), message instanceof Error ? message.toString() : message);
}
exports.error = error2;
function warning(message, properties = {}) {
function warning2(message, properties = {}) {
command_1.issueCommand("warning", utils_1.toCommandProperties(properties), message instanceof Error ? message.toString() : message);
}
exports.warning = warning;
exports.warning = warning2;
function notice(message, properties = {}) {
command_1.issueCommand("notice", utils_1.toCommandProperties(properties), message instanceof Error ? message.toString() : message);
}
exports.notice = notice;
function info2(message) {
function info3(message) {
process.stdout.write(message + os.EOL);
}
exports.info = info2;
function startGroup(name) {
exports.info = info3;
function startGroup2(name) {
command_1.issue("group", name);
}
exports.startGroup = startGroup;
function endGroup() {
exports.startGroup = startGroup2;
function endGroup2() {
command_1.issue("endgroup");
}
exports.endGroup = endGroup;
function group(name, fn) {
exports.endGroup = endGroup2;
function group2(name, fn) {
return __awaiter(this, void 0, void 0, function* () {
startGroup(name);
startGroup2(name);
let result;
try {
result = yield fn();
} finally {
endGroup();
endGroup2();
}
return result;
});
}
exports.group = group;
exports.group = group2;
function saveState(name, value) {
command_1.issueCommand("save-state", { name }, value);
}
Expand Down Expand Up @@ -15552,22 +15552,29 @@ async function revokeActiveInstallationToken(githubOrToken) {

//
var reposToSync = core.getMultilineInput("repos", { required: true, trimWhitespace: true });
var fileToSync = core.getMultilineInput("files", { required: true, trimWhitespace: true });
core.group("Repos being synced:", async () => reposToSync.forEach((repo) => `- ${repo}`));
var filesToSync = core.getMultilineInput("files", { required: true, trimWhitespace: true });
core.group("Files being synced:", async () => filesToSync.forEach((file) => `- ${file}`));
async function getFilesForRepo(github, repo) {
core.startGroup(`Retrieving files from "${repo}" repo`);
const fileMap = /* @__PURE__ */ new Map();
for (const path of fileToSync) {
for (const path of filesToSync) {
fileMap.set(path, await getFile(github, repo, path));
}
core.info(`Retrieved ${fileMap.size} file(s)`);
core.endGroup();
return fileMap;
}
async function getFile(github, repo, path) {
core.info(`Retrieving "${path}" from ${repo} repo`);
return github.rest.repos.getContent({ owner: import_github2.context.repo.owner, repo, path }).then((response) => {
if (response.data.content !== void 0) {
return response.data;
}
return null;
}, (reason) => {
if (reason.status === 404) {
core.warning(`"${path}" does not exist in "${repo}" repo`);
return null;
}
throw reason;
Expand All @@ -15581,13 +15588,14 @@ async function updateRepoWithFiles(github, repo, goldenFiles) {
return;
}
const repoFile = repoFiles.get(path) || null;
let repoSha;
let repoFileContent;
let repoSha = void 0;
let repoFileContent = void 0;
if (repoFile !== null) {
repoSha = repoFile.sha;
repoFileContent = repoFile.content;
}
if (repoFileContent !== goldenFile.content) {
core.info(`Updating "${path}" in "${repo}" repo`);
updates.push(github.repos.createOrUpdateFileContents({
content: goldenFile.content,
owner: import_github2.context.repo.owner,
Expand All @@ -15596,6 +15604,8 @@ async function updateRepoWithFiles(github, repo, goldenFiles) {
message: `build: update \`${path}\` to match the content of \`${import_github2.context.repo.owner}/${import_github2.context.repo.repo}\``,
sha: repoSha
}));
} else {
core.info(`"${path}" is already in sync`);
}
});
await Promise.all(updates);
Expand All @@ -15605,6 +15615,7 @@ async function main() {
try {
const goldenFiles = await getFilesForRepo(github, import_github2.context.repo.repo);
for (const repo of reposToSync) {
core.info(`~~~~~~Updating "${repo}" repo~~~~~~~`);
await updateRepoWithFiles(github, repo, goldenFiles);
}
} finally {
Expand Down
19 changes: 15 additions & 4 deletions github-actions/org-file-sync/src/main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import {RequestError} from '@octokit/types';
import {getAuthTokenFor, ANGULAR_ROBOT, revokeActiveInstallationToken} from '../../utils.js';

const reposToSync = core.getMultilineInput('repos', {required: true, trimWhitespace: true});
const fileToSync = core.getMultilineInput('files', {required: true, trimWhitespace: true});
core.group('Repos being synced:', async () => reposToSync.forEach((repo) => `- ${repo}`));
const filesToSync = core.getMultilineInput('files', {required: true, trimWhitespace: true});
core.group('Files being synced:', async () => filesToSync.forEach((file) => `- ${file}`));

/**
* A file to be synced, a custom interface is used due to Octokit's types not properly expressing
Expand All @@ -21,10 +23,13 @@ type Files = Map<string, File | null>;

/** Retrieve the files from Github which are syncronized for a given repo. */
async function getFilesForRepo(github: Octokit, repo: string): Promise<Files> {
core.startGroup(`Retrieving files from "${repo}" repo`);
const fileMap = new Map<string, File | null>();
for (const path of fileToSync) {
for (const path of filesToSync) {
fileMap.set(path, await getFile(github, repo, path));
}
core.info(`Retrieved ${fileMap.size} file(s)`);
core.endGroup();
return fileMap;
}

Expand All @@ -33,6 +38,7 @@ async function getFilesForRepo(github: Octokit, repo: string): Promise<Files> {
* repo and returning a 404.
*/
async function getFile(github: Octokit, repo: string, path: string): Promise<File | null> {
core.info(`Retrieving "${path}" from ${repo} repo`);
return github.rest.repos.getContent({owner: context.repo.owner, repo, path}).then(
(response) => {
if ((response.data as {content?: string}).content !== undefined) {
Expand All @@ -42,6 +48,7 @@ async function getFile(github: Octokit, repo: string, path: string): Promise<Fil
},
(reason: RequestError) => {
if (reason.status === 404) {
core.warning(`"${path}" does not exist in "${repo}" repo`);
return null;
}
throw reason;
Expand Down Expand Up @@ -70,9 +77,9 @@ async function updateRepoWithFiles(github: Octokit, repo: string, goldenFiles: F
/** The target repository's File for the path. */
const repoFile = repoFiles.get(path) || null;
/** The SHA of the last time the file was updated in the target repo. */
let repoSha: string | undefined;
let repoSha: string | undefined = undefined;
/** The current content of the file in the target repo. */
let repoFileContent: string | undefined;
let repoFileContent: string | undefined = undefined;

// If the repo file is null, there is not previous information to use for comparisons
if (repoFile !== null) {
Expand All @@ -81,6 +88,7 @@ async function updateRepoWithFiles(github: Octokit, repo: string, goldenFiles: F
}

if (repoFileContent !== goldenFile.content) {
core.info(`Updating "${path}" in "${repo}" repo`);
updates.push(
github.repos.createOrUpdateFileContents({
content: goldenFile.content,
Expand All @@ -92,6 +100,8 @@ async function updateRepoWithFiles(github: Octokit, repo: string, goldenFiles: F
sha: repoSha,
}),
);
} else {
core.info(`"${path}" is already in sync`);
}
});

Expand All @@ -104,6 +114,7 @@ async function main() {
const goldenFiles: Files = await getFilesForRepo(github, context.repo.repo);

for (const repo of reposToSync) {
core.info(`~~~~~~Updating "${repo}" repo~~~~~~~`);
await updateRepoWithFiles(github, repo, goldenFiles);
}
} finally {
Expand Down

0 comments on commit 9db43e7

Please sign in to comment.