Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix another "cannot read property of undefined" error #1354

Merged
merged 2 commits into from
Sep 25, 2024
Merged

Fix another "cannot read property of undefined" error #1354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
529eb94
Fix reading undefined reference when there are no existing pull reque…
rhyskoedijk Sep 24, 2024
c61b8b0
Improved logging to ensure error stack traces are always dumped; help…
rhyskoedijk Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 20 additions & 18 deletions extension/tasks/dependabotV2/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,25 +111,27 @@ async function run() {
failedJobs++;
}

// Run an update job for each existing pull request; this will resolve merge conflicts and close pull requests that are no longer needed
if (!taskInputs.skipPullRequests) {
for (const pullRequestId in existingPullRequests) {
const updatePullRequestJob = DependabotJobBuilder.newUpdatePullRequestJob(
taskInputs,
pullRequestId,
update,
dependabotConfig.registries,
existingPullRequestDependencies,
existingPullRequests[pullRequestId],
);
const updatePullRequestOutputs = await dependabot.update(updatePullRequestJob, dependabotUpdaterOptions);
if (!updatePullRequestOutputs || updatePullRequestOutputs.filter((u) => !u.success).length > 0) {
updatePullRequestOutputs.filter((u) => !u.success).forEach((u) => exception(u.error));
failedJobs++;
// If there are existing pull requests, run an update job for each one; this will resolve merge conflicts and close pull requests that are no longer needed
if (existingPullRequests && existingPullRequests.keys.length > 0) {
Copy link
Contributor Author

@rhyskoedijk rhyskoedijk Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@mburumaxwell sorry, I had a brain fart here, this line should be:

if (existingPullRequests && Object.keys(existingPullRequests).length > 0) {

Are you able to edit in main? Otherwise I will make a PR to update it later today.
I really need to get started on those unit tests 😓

Copy link
Contributor

Choose a reason for hiding this comment

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

Haven't heard "brain fart" before 😸
7b0ed03

if (!taskInputs.skipPullRequests) {
for (const pullRequestId in existingPullRequests) {
const updatePullRequestJob = DependabotJobBuilder.newUpdatePullRequestJob(
taskInputs,
pullRequestId,
update,
dependabotConfig.registries,
existingPullRequestDependencies,
existingPullRequests[pullRequestId],
);
const updatePullRequestOutputs = await dependabot.update(updatePullRequestJob, dependabotUpdaterOptions);
if (!updatePullRequestOutputs || updatePullRequestOutputs.filter((u) => !u.success).length > 0) {
updatePullRequestOutputs.filter((u) => !u.success).forEach((u) => exception(u.error));
failedJobs++;
}
}
} else {
warning(`Skipping update of existing pull requests as 'skipPullRequests' is set to 'true'`);
}
} else if (existingPullRequests.keys.length > 0) {
warning(`Skipping update of existing pull requests as 'skipPullRequests' is set to 'true'`);
}
}

Expand All @@ -150,7 +152,7 @@ async function run() {
function exception(e: Error) {
if (e) {
error(`An unhandled exception occurred: ${e}`);
console.error(e);
console.debug(e); // Dump the stack trace to help with debugging
}
}

Expand Down
16 changes: 8 additions & 8 deletions extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class AzureDevOpsWebApiClient {
return repo.defaultBranch;
} catch (e) {
error(`Failed to get default branch for '${project}/${repository}': ${e}`);
console.error(e);
console.debug(e); // Dump the error stack trace to help with debugging
return undefined;
}
}
Expand Down Expand Up @@ -114,7 +114,7 @@ export class AzureDevOpsWebApiClient {
);
} catch (e) {
error(`Failed to list active pull request properties: ${e}`);
console.error(e);
console.debug(e); // Dump the error stack trace to help with debugging
return [];
}
}
Expand Down Expand Up @@ -264,7 +264,7 @@ export class AzureDevOpsWebApiClient {
return pullRequest.pullRequestId;
} catch (e) {
error(`Failed to create pull request: ${e}`);
console.error(e);
console.debug(e); // Dump the error stack trace to help with debugging
return null;
}
}
Expand Down Expand Up @@ -348,7 +348,7 @@ export class AzureDevOpsWebApiClient {
return true;
} catch (e) {
error(`Failed to update pull request: ${e}`);
console.error(e);
console.debug(e); // Dump the error stack trace to help with debugging
return false;
}
}
Expand Down Expand Up @@ -384,7 +384,7 @@ export class AzureDevOpsWebApiClient {
console.info(` - Pull request #${options.pullRequestId} was approved.`);
} catch (e) {
error(`Failed to approve pull request: ${e}`);
console.error(e);
console.debug(e); // Dump the error stack trace to help with debugging
return false;
}
}
Expand Down Expand Up @@ -462,7 +462,7 @@ export class AzureDevOpsWebApiClient {
return true;
} catch (e) {
error(`Failed to close pull request: ${e}`);
console.error(e);
console.debug(e); // Dump the error stack trace to help with debugging
return false;
}
}
Expand All @@ -482,7 +482,7 @@ export class AzureDevOpsWebApiClient {
return properties.map((p) => ({ [p.name]: p.value })).reduce((a, b) => ({ ...a, ...b }), {});
} catch (e) {
error(`Failed to get project properties: ${e}`);
console.error(e);
console.debug(e); // Dump the error stack trace to help with debugging
return undefined;
}
}
Expand Down Expand Up @@ -517,7 +517,7 @@ export class AzureDevOpsWebApiClient {
]);
} catch (e) {
error(`Failed to update project property '${name}': ${e}`);
console.error(e);
console.debug(e); // Dump the error stack trace to help with debugging
}
}
}
Expand Down
6 changes: 2 additions & 4 deletions extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,13 +224,11 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess
break;

case 'record_update_job_error':
error(`Update job error: ${data['error-type']}`);
console.log(data['error-details']);
error(`Update job error: ${data['error-type']} ${JSON.stringify(data['error-details'])}`);
return false;

case 'record_update_job_unknown_error':
error(`Update job unknown error: ${data['error-type']}`);
console.log(data['error-details']);
error(`Update job unknown error: ${data['error-type']}, ${JSON.stringify(data['error-details'])}`);
return false;

case 'increment_metric':
Expand Down