Skip to content

Commit

Permalink
fix: more error handling (#2541)
Browse files Browse the repository at this point in the history
* fix: error handling for artifact fetch
     error handling during PR webhook processing

* fix: add test for webhook change

* fix: add test for artifact error handling
  • Loading branch information
jithine authored Aug 26, 2021
1 parent c5fd3b1 commit 4d3dfcc
Show file tree
Hide file tree
Showing 5 changed files with 196 additions and 133 deletions.
62 changes: 31 additions & 31 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,29 +68,29 @@
],
"dependencies": {
"@hapi/bell": "^12.1.1",
"@hapi/boom": "^9.1.1",
"@hapi/boom": "^9.1.4",
"@hapi/cookie": "^11.0.2",
"@hapi/crumb": "^8.0.1",
"@hapi/good": "^9.0.1",
"@hapi/good-console": "^9.0.1",
"@hapi/good-squeeze": "^6.0.0",
"@hapi/hapi": "^20.1.4",
"@hapi/hapi": "^20.1.5",
"@hapi/hoek": "^9.1.1",
"@hapi/inert": "^6.0.3",
"@hapi/vision": "^6.1.0",
"@promster/hapi": "^4.2.1",
"async": "^3.2.0",
"async": "^3.2.1",
"badge-maker": "^3.3.1",
"config": "^1.31.0",
"date-fns": "^1.30.1",
"dayjs": "^1.10.4",
"dayjs": "^1.10.6",
"got": "^11.8.2",
"hapi-auth-bearer-token": "^8.0.0",
"hapi-auth-jwt2": "^10.2.0",
"hapi-rate-limit": "^5.0.0",
"hapi-swagger": "^14.2.0",
"hapi-swagger": "^14.2.4",
"ioredis": "^4.27.8",
"joi": "^17.4.0",
"joi": "^17.4.2",
"js-yaml": "^3.14.1",
"jsonwebtoken": "^8.5.1",
"license-checker": "^17.0.0",
Expand All @@ -101,32 +101,32 @@
"redlock": "^4.1.0",
"request": "^2.88.2",
"requestretry": "^4.1.2",
"screwdriver-artifact-bookend": "^1.1.27",
"screwdriver-build-bookend": "^2.3.3",
"screwdriver-artifact-bookend": "^1.2.0",
"screwdriver-build-bookend": "^2.4.0",
"screwdriver-cache-bookend": "^2.0.1",
"screwdriver-command-validator": "^2.0.2",
"screwdriver-config-parser": "^7.1.3",
"screwdriver-command-validator": "^2.1.0",
"screwdriver-config-parser": "^7.2.0",
"screwdriver-coverage-bookend": "^1.0.3",
"screwdriver-coverage-sonar": "^3.1.4",
"screwdriver-data-schema": "^21.3.1",
"screwdriver-datastore-sequelize": "^7.2.6",
"screwdriver-executor-base": "^8.1.2",
"screwdriver-coverage-sonar": "^3.3.0",
"screwdriver-data-schema": "^21.8.0",
"screwdriver-datastore-sequelize": "^7.2.7",
"screwdriver-executor-base": "^8.4.0",
"screwdriver-executor-docker": "^5.0.1",
"screwdriver-executor-k8s": "^14.10.1",
"screwdriver-executor-k8s": "^14.14.0",
"screwdriver-executor-k8s-vm": "^4.3.2",
"screwdriver-executor-queue": "^3.0.2",
"screwdriver-executor-router": "^2.1.2",
"screwdriver-logger": "^1.0.2",
"screwdriver-models": "^28.8.0",
"screwdriver-notifications-email": "^2.1.5",
"screwdriver-notifications-slack": "^3.1.6",
"screwdriver-scm-base": "^7.2.0",
"screwdriver-scm-bitbucket": "^4.2.0",
"screwdriver-scm-github": "^11.3.0",
"screwdriver-scm-gitlab": "^2.4.0",
"screwdriver-scm-router": "^6.2.0",
"screwdriver-template-validator": "^5.1.2",
"screwdriver-workflow-parser": "^3.1.3",
"screwdriver-executor-router": "^2.3.0",
"screwdriver-logger": "^1.1.0",
"screwdriver-models": "^28.8.5",
"screwdriver-notifications-email": "^2.2.0",
"screwdriver-notifications-slack": "^3.2.0",
"screwdriver-scm-base": "^7.3.0",
"screwdriver-scm-bitbucket": "^4.5.0",
"screwdriver-scm-github": "^11.6.0",
"screwdriver-scm-gitlab": "^2.7.0",
"screwdriver-scm-router": "^6.3.0",
"screwdriver-template-validator": "^5.2.0",
"screwdriver-workflow-parser": "^3.2.0",
"sqlite3": "^5.0.1",
"tinytim": "^0.1.1",
"uuid": "^8.3.2",
Expand All @@ -139,15 +139,15 @@
}
},
"devDependencies": {
"@octokit/rest": "^18.6.0",
"@octokit/rest": "^18.9.1",
"chai": "~3.5.0",
"chai-as-promised": "^6.0.0",
"chai-jwt": "^2.0.0",
"coveralls": "^3.1.0",
"coveralls": "^3.1.1",
"cucumber": "6.0.4",
"cucumber-pretty": "^6.0.0",
"eslint": "^7.26.0",
"eslint-config-screwdriver": "^5.0.5",
"eslint": "^7.32.0",
"eslint-config-screwdriver": "^5.0.7",
"form-data": "^2.5.1",
"mocha": "^8.4.0",
"mocha-multi-reporters": "^1.5.1",
Expand Down
18 changes: 13 additions & 5 deletions plugins/builds/artifacts/get.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const joi = require('joi');
const schema = require('screwdriver-data-schema');
const uuid = require('uuid');
const got = require('got');
const boom = require('@hapi/boom');

const idSchema = schema.models.build.base.extract('id');
const artifactSchema = joi.string().label('Artifact Name');
Expand Down Expand Up @@ -54,22 +55,29 @@ module.exports = config => ({
expiresIn: '5s',
jwtid: uuid.v4()
});

let baseUrl = `${config.ecosystem.store}/v1/builds/`
+ `${buildId}/ARTIFACTS/${encodedArtifact}?token=${token}`;

if (request.query.type) {
baseUrl += `&type=${request.query.type}`;
}

const gotStream = got.stream(baseUrl);

let response = h.response(gotStream);
return new Promise((resolve) => {

return new Promise((resolve, reject) => {
gotStream.on('response', response => {
resolve(response.headers);
});
});
gotStream.on('error', err => {
if (err.response && err.response.statusCode == 404) {
reject(boom.notFound('File not found'));
} else {
reject(err);
}
});
}).then(headers => {
response.headers['content-type'] = headers['content-type'];
response.headers['content-disposition'] = headers['content-disposition'];
Expand Down
176 changes: 91 additions & 85 deletions plugins/webhooks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -397,116 +397,122 @@ async function createPREvents(options, request) {

const eventConfigs = await Promise.all(
pipelines.map(async p => {
const b = await p.branch;
// obtain pipeline's latest commit sha for branch specific job
let configPipelineSha = '';
let subscribedConfigSha = '';
let eventConfig = {};
try {
const b = await p.branch;
// obtain pipeline's latest commit sha for branch specific job
let configPipelineSha = '';
let subscribedConfigSha = '';
let eventConfig = {};

// Check if the webhook event is from a subscribed repo and
// and fetch the source repo commit sha and save the subscribed sha
if (uriTrimmer(scmConfig.scmUri) !== uriTrimmer(p.scmUri)) {
subscribedConfigSha = sha;
// Check if the webhook event is from a subscribed repo and
// and fetch the source repo commit sha and save the subscribed sha
if (uriTrimmer(scmConfig.scmUri) !== uriTrimmer(p.scmUri)) {
subscribedConfigSha = sha;

try {
sha = await pipelineFactory.scm.getCommitSha({
scmUri: p.scmUri,
scmContext: scmConfig.scmContext,
token: scmConfig.token
});
} catch (err) {
if (err.status >= 500) {
throw err;
} else {
logger.info(`skip create event for branch: ${b}`);
try {
sha = await pipelineFactory.scm.getCommitSha({
scmUri: p.scmUri,
scmContext: scmConfig.scmContext,
token: scmConfig.token
});
} catch (err) {
if (err.status >= 500) {
throw err;
} else {
logger.info(`skip create event for branch: ${b}`);
}
}
}

configPipelineSha = sha;
} else {
try {
configPipelineSha = await pipelineFactory.scm.getCommitSha(scmConfig);
} catch (err) {
if (err.status >= 500) {
throw err;
} else {
logger.info(`skip create event for branch: ${b}`);
configPipelineSha = sha;
} else {
try {
configPipelineSha = await pipelineFactory.scm.getCommitSha(scmConfig);
} catch (err) {
if (err.status >= 500) {
throw err;
} else {
logger.info(`skip create event for branch: ${b}`);
}
}
}
}

const { skipMessage, resolvedChainPR } = getSkipMessageAndChainPR({
// Workaround for pipelines which has NULL value in `pipeline.annotations`
pipeline: !p.annotations ? { annotations: {}, ...p } : p,
prSource,
restrictPR,
chainPR
});
const { skipMessage, resolvedChainPR } = getSkipMessageAndChainPR({
// Workaround for pipelines which has NULL value in `pipeline.annotations`
pipeline: !p.annotations ? { annotations: {}, ...p } : p,
prSource,
restrictPR,
chainPR
});

const prInfo = await eventFactory.scm.getPrInfo(scmConfig);
const prInfo = await eventFactory.scm.getPrInfo(scmConfig);

eventConfig = {
pipelineId: p.id,
type: 'pr',
webhooks: true,
username,
scmContext: scmConfig.scmContext,
sha,
configPipelineSha,
startFrom: `~pr:${branch}`,
changedFiles,
causeMessage: `${action} by ${userDisplayName}`,
chainPR: resolvedChainPR,
prRef,
prNum,
prTitle,
prInfo,
prSource,
baseBranch: branch
};

if (b === branch) {
eventConfig.startFrom = '~pr';
}

// Check if the webhook event is from a subscribed repo and
// set the jobs entrypoint from ~startFrom
// For subscribed PR event, it should be mimicked as a commit
// in order to function properly
if (uriTrimmer(scmConfig.scmUri) !== uriTrimmer(p.scmUri)) {
eventConfig = {
pipelineId: p.id,
type: 'pipeline',
type: 'pr',
webhooks: true,
username,
scmContext: scmConfig.scmContext,
startFrom: '~subscribe',
sha,
configPipelineSha,
startFrom: `~pr:${branch}`,
changedFiles,
baseBranch: branch,
causeMessage: `Merged by ${username}`,
meta,
releaseName,
ref,
subscribedEvent: true,
subscribedConfigSha,
subscribedSourceUrl: prInfo.url
causeMessage: `${action} by ${userDisplayName}`,
chainPR: resolvedChainPR,
prRef,
prNum,
prTitle,
prInfo,
prSource,
baseBranch: branch
};

await updateAdmins(userFactory, username, scmConfig.scmContext, p.id, pipelineFactory);
}
if (b === branch) {
eventConfig.startFrom = '~pr';
}

if (skipMessage) {
eventConfig.skipMessage = skipMessage;
}
// Check if the webhook event is from a subscribed repo and
// set the jobs entrypoint from ~startFrom
// For subscribed PR event, it should be mimicked as a commit
// in order to function properly
if (uriTrimmer(scmConfig.scmUri) !== uriTrimmer(p.scmUri)) {
eventConfig = {
pipelineId: p.id,
type: 'pipeline',
webhooks: true,
username,
scmContext: scmConfig.scmContext,
startFrom: '~subscribe',
sha,
configPipelineSha,
changedFiles,
baseBranch: branch,
causeMessage: `Merged by ${username}`,
meta,
releaseName,
ref,
subscribedEvent: true,
subscribedConfigSha,
subscribedSourceUrl: prInfo.url
};

await updateAdmins(userFactory, username, scmConfig.scmContext, p.id, pipelineFactory);
}

if (skipMessage) {
eventConfig.skipMessage = skipMessage;
}

return eventConfig;
} catch (err) {
logger.warn(`pipeline:${p.id} error in starting event`, err);

return eventConfig;
return null;
}
})
);

eventConfigs.forEach(eventConfig => {
if (eventConfig.configPipelineSha) {
if (eventConfig && eventConfig.configPipelineSha) {
events.push(eventFactory.create(eventConfig));
}
});
Expand Down
Loading

0 comments on commit 4d3dfcc

Please sign in to comment.