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

Jump to bottom

add outer try-catch fix issue #545 #546

Merged
merged 2 commits into from
Dec 8, 2023
Merged

add outer try-catch fix issue #545 #546

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
8ec4892
add outer try-catch fix issue #545
decyjphr Oct 31, 2023
4998f97
ignore 404s in dry-run
decyjphr Nov 1, 2023
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
5 changes: 5 additions & 0 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,11 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
return
}

if (check_run.status === 'completed') {
robot.log.debug(' Checkrun created as completed, returning')
return
}

const adminRepo = repository.name === env.ADMIN_REPO
robot.log.debug(`Is Admin repo event ${adminRepo}`)
if (!adminRepo) {
Expand Down
12 changes: 9 additions & 3 deletions lib/plugins/diffable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,16 @@ module.exports = class Diffable extends ErrorStash {
}
return Promise.all(changes)
}).catch(e => {
this.logError(`Error ${e} in ${this.constructor.name} for repo: ${JSON.stringify(this.repo)} entries ${JSON.stringify(this.entries)}`)
if (this.nop) {
resArray.push(new NopCommand(this.constructor.name, this.repo, null, `error ${e} in ${this.constructor.name} for repo: ${JSON.stringify(this.repo)} entries ${JSON.stringify(this.entries)}`, 'ERROR'))
return Promise.resolve(resArray)
if (e.status === 404) {
// Ignore 404s which can happen in dry-run as the repo may not exist.
return Promise.resolve(resArray)
} else {
resArray.push(new NopCommand(this.constructor.name, this.repo, null, `error ${e} in ${this.constructor.name} for repo: ${JSON.stringify(this.repo)} entries ${JSON.stringify(this.entries)}`, 'ERROR'))
return Promise.resolve(resArray)
}
} else {
this.logError(`Error ${e} in ${this.constructor.name} for repo: ${JSON.stringify(this.repo)} entries ${JSON.stringify(this.entries)}`)
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion lib/plugins/errorStash.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module.exports = class ErrorStash {
this.errors = errors
}

async logError (msg) {
logError (msg) {
this.log.error(msg)
this.errors.push({
owner: this.repo.owner,
Expand Down
9 changes: 6 additions & 3 deletions lib/plugins/rulesets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const version = {
'X-GitHub-Api-Version': '2022-11-28'
}
module.exports = class Rulesets extends Diffable {
constructor (nop, github, repo, entries, log, scope, errors) {
constructor (nop, github, repo, entries, log, errors, scope) {
super(nop, github, repo, entries, log, errors)
this.github = github
this.repo = repo
Expand Down Expand Up @@ -152,7 +152,7 @@ module.exports = class Rulesets extends Diffable {
if (this.scope === 'org') {
if (this.nop) {
return Promise.resolve([
new NopCommand(this.constructor.name, this.repo, this.github.fetch.endpoint('DELETE /orgs/{org}/rulesets/{id}', parms), 'Delete Ruleset')
new NopCommand(this.constructor.name, this.repo, this.github.request.endpoint('DELETE /orgs/{org}/rulesets/{id}', parms), 'Delete Ruleset')
])
}
this.log.debug(`Deleting Ruleset with the following values ${JSON.stringify(parms, null, 2)}`)
Expand All @@ -165,14 +165,17 @@ module.exports = class Rulesets extends Diffable {
} else {
if (this.nop) {
return Promise.resolve([
new NopCommand(this.constructor.name, this.repo, this.github.fetch.endpoint('DELETE /repos/{owner}/{repo}/rulesets/{id}', parms), 'Delete Ruleset')
new NopCommand(this.constructor.name, this.repo, this.github.request.endpoint('DELETE /repos/{owner}/{repo}/rulesets/{id}', parms), 'Delete Ruleset')
])
}
this.log.debug(`Deleting Ruleset with the following values ${JSON.stringify(parms, null, 2)}`)
return this.github.request('DELETE /repos/{owner}/{repo}/rulesets/{id}', parms).then(res => {
this.log(`Ruleset deleted successfully ${JSON.stringify(res.url)}`)
return res
}).catch(e => {
if (e.status === 404) {
return
}
return this.handleError(e)
})
}
Expand Down
64 changes: 44 additions & 20 deletions lib/settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,43 @@ const SCOPE = { ORG: 'org', REPO: 'repo' } // Determine if the setting is a org
class Settings {
static async syncAll (nop, context, repo, config, ref) {
const settings = new Settings(nop, context, repo, config, ref)
await settings.loadConfigs()
// settings.repoConfigs = await settings.getRepoConfigs()
await settings.updateOrg()
await settings.updateAll()
await settings.handleResults()
try {
await settings.loadConfigs()
// settings.repoConfigs = await settings.getRepoConfigs()
await settings.updateOrg()
await settings.updateAll()
await settings.handleResults()
} catch (error) {
settings.logError(error.message)
await settings.handleResults()
}
}

static async syncSubOrgs (nop, context, suborg, repo, config, ref) {
const settings = new Settings(nop, context, repo, config, ref, suborg)
await settings.loadConfigs()
await settings.updateAll()
await settings.handleResults()
try {
await settings.loadConfigs()
await settings.updateAll()
await settings.handleResults()
} catch (error) {
settings.logError(error.message)
await settings.handleResults()
}
}

static async sync (nop, context, repo, config, ref) {
const settings = new Settings(nop, context, repo, config, ref)
await settings.loadConfigs(repo)
if (settings.isRestricted(repo.repo)) {
return
try {
await settings.loadConfigs(repo)
if (settings.isRestricted(repo.repo)) {
return
}
await settings.updateRepos(repo)
await settings.handleResults()
} catch (error) {
settings.logError(error.message)
await settings.handleResults()
}
await settings.updateRepos(repo)
await settings.handleResults()
}

static async handleError (nop, context, repo, config, ref, nopcommand) {
Expand Down Expand Up @@ -124,6 +139,16 @@ class Settings {
})
}

logError (msg) {
this.log.error(msg)
this.errors.push({
owner: this.repo.owner,
repo: this.repo.repo,
msg,
plugin: this.constructor.name
})
}

async handleResults () {
const { payload } = this.context

Expand Down Expand Up @@ -163,7 +188,7 @@ class Settings {
stats.errors[res.repo] = []
}
stats.errors[res.repo].push(res.action)
} else if (!(res.action.additions === null && res.action.deletions === null && res.action.modifications === null)) {
} else if (!(res.action?.additions === null && res.action?.deletions === null && res.action?.modifications === null)) {
if (!stats.changes[res.plugin]) {
stats.changes[res.plugin] = {}
}
Expand Down Expand Up @@ -201,9 +226,6 @@ ${this.results.reduce((x, y) => {
if (!y) {
return x
}
// if (y.endpoint) {
// return `${x}`
// // } else
if (y.type === 'ERROR') {
error = true
return `${x}
Expand Down Expand Up @@ -256,7 +278,7 @@ ${this.results.reduce((x, y) => {
const rulesetsConfig = this.config.rulesets
if (rulesetsConfig) {
const RulesetsPlugin = Settings.PLUGINS.rulesets
return new RulesetsPlugin(this.nop, this.github, this.repo, rulesetsConfig, this.log, SCOPE.ORG, this.errors).sync().then(res => {
return new RulesetsPlugin(this.nop, this.github, this.repo, rulesetsConfig, this.log, this.errors, SCOPE.ORG).sync().then(res => {
this.appendToResults(res)
})
}
Expand Down Expand Up @@ -772,6 +794,9 @@ ${this.results.reduce((x, y) => {
}

function prettify(obj) {
if (obj === null || obj === undefined) {
return ''
}
return JSON.stringify(obj, null, 2).replaceAll('\n', '<br>').replaceAll(' ', '&nbsp;')
}

Expand All @@ -786,8 +811,7 @@ Settings.PLUGINS = {
branches: require('./plugins/branches'),
autolinks: require('./plugins/autolinks'),
validator: require('./plugins/validator'),
rulesets: require('./plugins/rulesets'),
environments: require('./plugins/environments')
rulesets: require('./plugins/rulesets')
Comment on lines -789 to +814
Copy link
Contributor

@svg153 svg153 Dec 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Why this change? Did you remove the environments plugin ?

Copy link
Contributor

@svg153 svg153 Dec 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@decyjphr you have merged this PR with this change. The envs were added in the prev PR #544 in this line. Should rebasing branch before merge be enabled for this repository?

Copy link
Contributor

Choose a reason for hiding this comment

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

PR to add envs again: #567

}

module.exports = Settings
1 change: 1 addition & 0 deletions test/unit/lib/mergeDeep.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ describe('MergeDeep Test', () => {
it('CompareDeep extensive test', () => {
const target = YAML.load(`
repository:
name: test
# A short description of the repository that will show up on GitHub
description: description of the repos

Expand Down
Loading