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: create mirror error handling overwriting initial error #301

Merged
merged 1 commit into from
Jan 21, 2025
Merged

fix: create mirror error handling overwriting initial error #301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
162 changes: 84 additions & 78 deletions src/server/repos/controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ export const createMirrorHandler = async ({
}: {
input: CreateMirrorSchema
}) => {
// Use to track mirror creation for cleanup in case of errors
let privateOctokit
let newRepo

try {
reposApiLogger.info('createMirror', { input: input })

Expand All @@ -41,7 +45,7 @@ export const createMirrorHandler = async ({
const contributionOctokit = octokitData.contribution.octokit
const contributionAccessToken = octokitData.contribution.accessToken

const privateOctokit = octokitData.private.octokit
privateOctokit = octokitData.private.octokit
const privateInstallationId = octokitData.private.installationId
const privateAccessToken = octokitData.private.accessToken

Expand Down Expand Up @@ -79,95 +83,85 @@ export const createMirrorHandler = async ({
}
})

try {
const forkData = await contributionOctokit.rest.repos.get({
owner: input.forkRepoOwner,
repo: input.forkRepoName,
})
const forkData = await contributionOctokit.rest.repos.get({
owner: input.forkRepoOwner,
repo: input.forkRepoName,
})

// Now create a temporary directory to clone the repo into
const tempDir = temporaryDirectory()

const options: Partial<SimpleGitOptions> = {
config: [
`user.name=pma[bot]`,
// We want to use the private installation ID as the email so that we can push to the private repo
`user.email=${privateInstallationId}+pma[bot]@users.noreply.github.com`,
// Disable any global git hooks to prevent potential interference when running the app locally
'core.hooksPath=/dev/null',
],
}
const git = simpleGit(tempDir, options)
const remote = generateAuthUrl(
contributionAccessToken,
input.forkRepoOwner,
input.forkRepoName,
)
// Now create a temporary directory to clone the repo into
const tempDir = temporaryDirectory()

const options: Partial<SimpleGitOptions> = {
config: [
`user.name=pma[bot]`,
// We want to use the private installation ID as the email so that we can push to the private repo
`user.email=${privateInstallationId}+pma[bot]@users.noreply.github.com`,
// Disable any global git hooks to prevent potential interference when running the app locally
'core.hooksPath=/dev/null',
],
}
const git = simpleGit(tempDir, options)
const remote = generateAuthUrl(
contributionAccessToken,
input.forkRepoOwner,
input.forkRepoName,
)

await git.clone(remote, tempDir)
await git.clone(remote, tempDir)

// Get the organization custom properties
const orgCustomProps =
await privateOctokit.rest.orgs.getAllCustomProperties({
org: privateOrg,
})

// Creates custom property fork in the org if it doesn't exist
if (
!orgCustomProps.data.some(
(prop: { property_name: string }) => prop.property_name === 'fork',
)
) {
await privateOctokit.rest.orgs.createOrUpdateCustomProperty({
org: privateOrg,
custom_property_name: 'fork',
value_type: 'string',
})
}
// Get the organization custom properties
const orgCustomProps =
await privateOctokit.rest.orgs.getAllCustomProperties({
org: privateOrg,
})

// This repo needs to be created in the private org
const newRepo = await privateOctokit.rest.repos.createInOrg({
name: input.newRepoName,
// Creates custom property fork in the org if it doesn't exist
if (
!orgCustomProps.data.some(
(prop: { property_name: string }) => prop.property_name === 'fork',
)
) {
await privateOctokit.rest.orgs.createOrUpdateCustomProperty({
org: privateOrg,
private: true,
description: `Mirror of ${input.forkRepoOwner}/${input.forkRepoName}`,
custom_properties: {
fork: `${input.forkRepoOwner}/${input.forkRepoName}`,
},
custom_property_name: 'fork',
value_type: 'string',
})
}

const defaultBranch = forkData.data.default_branch
// This repo needs to be created in the private org
newRepo = await privateOctokit.rest.repos.createInOrg({
name: input.newRepoName,
org: privateOrg,
private: true,
description: `Mirror of ${input.forkRepoOwner}/${input.forkRepoName}`,
custom_properties: {
fork: `${input.forkRepoOwner}/${input.forkRepoName}`,
},
})

// Add the mirror remote
const upstreamRemote = generateAuthUrl(
privateAccessToken,
newRepo.data.owner.login,
newRepo.data.name,
)
await git.addRemote('upstream', upstreamRemote)
await git.push('upstream', defaultBranch)
const defaultBranch = forkData.data.default_branch

// Create a new branch on both
await git.checkoutBranch(input.newBranchName, defaultBranch)
await git.push('origin', input.newBranchName)
// Add the mirror remote
const upstreamRemote = generateAuthUrl(
privateAccessToken,
newRepo.data.owner.login,
newRepo.data.name,
)
await git.addRemote('upstream', upstreamRemote)
await git.push('upstream', defaultBranch)

reposApiLogger.info('Mirror created', {
org: newRepo.data.owner.login,
name: newRepo.data.name,
})
// Create a new branch on both
await git.checkoutBranch(input.newBranchName, defaultBranch)
await git.push('origin', input.newBranchName)

return {
success: true,
data: newRepo.data,
}
} catch (error) {
// Clean up the private mirror repo made
await privateOctokit.rest.repos.delete({
owner: privateOrg,
repo: input.newRepoName,
})
reposApiLogger.info('Mirror created', {
org: newRepo.data.owner.login,
name: newRepo.data.name,
})

throw error
return {
success: true,
data: newRepo.data,
}
} catch (error) {
reposApiLogger.error('Error creating mirror', { error })
Expand All @@ -178,6 +172,18 @@ export const createMirrorHandler = async ({
(error as Error)?.message ??
'An error occurred'

if (privateOctokit && newRepo) {
try {
// Clean up the private mirror repo made
await privateOctokit.rest.repos.delete({
owner: newRepo.data.owner.login,
repo: input.newRepoName,
})
} catch (deleteError) {
reposApiLogger.error('Failed to delete mirror', { deleteError })
}
}

throw new TRPCError({
code: 'INTERNAL_SERVER_ERROR',
message,
Expand Down
29 changes: 24 additions & 5 deletions test/server/repos.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,9 +199,18 @@ describe('Repos router', () => {
om.mockFunctions.rest.orgs.get.mockResolvedValue(fakeOrg)
om.mockFunctions.rest.repos.get.mockResolvedValueOnce(repoNotFound)
om.mockFunctions.rest.repos.get.mockResolvedValueOnce(fakeMirrorRepo)
om.mockFunctions.rest.repos.delete.mockResolvedValue({})
stubbedGit.clone.mockResolvedValue({})
om.mockFunctions.rest.orgs.getAllCustomProperties.mockResolvedValue({
data: [{ fork: 'test' }],
})
om.mockFunctions.rest.repos.createInOrg.mockResolvedValue({
data: { owner: { login: 'github' } },
})

stubbedGit.clone.mockRejectedValue(new Error('clone error'))
// error after repo creation so that cleanup can be tested
stubbedGit.addRemote.mockRejectedValue(new Error('error adding remote'))

om.mockFunctions.rest.repos.delete.mockResolvedValue({})

await caller
.createMirror({
Expand All @@ -213,7 +222,7 @@ describe('Repos router', () => {
newRepoName: 'test',
})
.catch((error) => {
expect(error.message).toEqual('clone error')
expect(error.message).toEqual('error adding remote')
})

expect(configSpy).toHaveBeenCalledTimes(1)
Expand All @@ -239,9 +248,19 @@ describe('Repos router', () => {
om.mockFunctions.rest.orgs.get.mockResolvedValue(fakeOrg)
om.mockFunctions.rest.repos.get.mockResolvedValueOnce(repoNotFound)
om.mockFunctions.rest.repos.get.mockResolvedValueOnce(fakeMirrorRepo)
stubbedGit.clone.mockResolvedValue({})
om.mockFunctions.rest.orgs.getAllCustomProperties.mockResolvedValue({
data: [{ fork: 'test' }],
})
om.mockFunctions.rest.repos.createInOrg.mockResolvedValue({
data: { owner: { login: 'github-test' } },
})
om.mockFunctions.rest.repos.delete.mockResolvedValue({})

stubbedGit.clone.mockRejectedValue(new Error('clone error'))
// error after repo creation so that cleanup can be tested
stubbedGit.addRemote.mockRejectedValue(new Error('error adding remote'))

om.mockFunctions.rest.repos.delete.mockResolvedValue({})

await caller
.createMirror({
Expand All @@ -253,7 +272,7 @@ describe('Repos router', () => {
newRepoName: 'test',
})
.catch((error) => {
expect(error.message).toEqual('clone error')
expect(error.message).toEqual('error adding remote')
})

expect(configSpy).toHaveBeenCalledTimes(1)
Expand Down
Loading