Skip to content

Commit 28d3dc8

Browse files
committed
Don't swallow any exceptions in CheckpointService
1 parent dcebebb commit 28d3dc8

File tree

2 files changed

+41
-61
lines changed

2 files changed

+41
-61
lines changed

src/services/checkpoints/CheckpointService.ts

Lines changed: 41 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -61,49 +61,34 @@ export class CheckpointService {
6161
) {}
6262

6363
private async pushStash() {
64-
try {
65-
const status = await this.git.status()
64+
const status = await this.git.status()
6665

67-
if (status.files.length > 0) {
68-
await this.git.stash(["-u"]) // Stash tracked and untracked files.
69-
return true
70-
} else {
71-
return undefined
72-
}
73-
} catch (err) {
74-
this.log(`[pushStash] Failed to stash changes: ${err instanceof Error ? err.message : String(err)}`)
66+
if (status.files.length > 0) {
67+
await this.git.stash(["-u"]) // Stash tracked and untracked files.
68+
return true
69+
} else {
7570
return false
7671
}
7772
}
7873

7974
private async applyStash() {
80-
try {
81-
const stashList = await this.git.stashList()
75+
const stashList = await this.git.stashList()
8276

83-
if (stashList.all.length > 0) {
84-
await this.git.stash(["apply"]) // Apply the most recent stash.
85-
return true
86-
} else {
87-
return undefined
88-
}
89-
} catch (err) {
90-
this.log(`[applyStash] Failed to apply stash: ${err instanceof Error ? err.message : String(err)}`)
77+
if (stashList.all.length > 0) {
78+
await this.git.stash(["apply"]) // Apply the most recent stash.
79+
return true
80+
} else {
9181
return false
9282
}
9383
}
9484

9585
private async popStash() {
96-
try {
97-
const stashList = await this.git.stashList()
86+
const stashList = await this.git.stashList()
9887

99-
if (stashList.all.length > 0) {
100-
await this.git.stash(["pop"]) // Pop the most recent stash.
101-
return true
102-
} else {
103-
return undefined
104-
}
105-
} catch (err) {
106-
this.log(`[popStash] Failed to pop stash: ${err instanceof Error ? err.message : String(err)}`)
88+
if (stashList.all.length > 0) {
89+
await this.git.stash(["pop"]) // Pop the most recent stash.
90+
return true
91+
} else {
10792
return false
10893
}
10994
}
@@ -141,27 +126,31 @@ export class CheckpointService {
141126
return undefined
142127
}
143128

144-
try {
145-
// Get the latest commit on the hidden branch before we reset it.
146-
const latestHash = await this.git.revparse([this.hiddenBranch]) // git rev-parse <hiddenBranch>
129+
// Get the latest commit on the hidden branch before we reset it.
130+
const latestHash = await this.git.revparse([this.hiddenBranch]) // git rev-parse <hiddenBranch>
131+
await this.git.checkout(this.hiddenBranch) // git checkout <hiddenBranch>
147132

133+
const reset = async () => {
134+
await this.git.reset(["HEAD", "."]) // git reset HEAD .
135+
await this.git.clean([CleanOptions.FORCE, CleanOptions.RECURSIVE]) // git clean -f -d
136+
await this.git.reset(["--hard", latestHash]) // git reset --hard <latestHash>
137+
await this.git.checkout(this.mainBranch) // git checkout <mainBranch>
138+
await this.popStash() // git stash pop
139+
}
140+
141+
try {
148142
// Reset hidden branch to match main and apply the pending changes.
149-
await this.git.checkout(this.hiddenBranch) // git checkout <hiddenBranch>
150143
await this.git.reset(["--hard", this.mainBranch]) // git reset --hard <mainBranch>
151144
await this.applyStash() // git stash apply 0
152145

153-
// If the diff is empty and there are no untracked files then we
154-
// don't need to commit.
155146
await this.git.add(["."]) // git add .
156147
const diff = await this.git.diff([latestHash]) // git diff <latestHash>
157148

158149
if (!diff) {
150+
// If the diff is empty and there are no untracked files then we
151+
// don't need to commit.
159152
this.log(`[saveCheckpoint] Diff is empty, no untracked files`)
160-
await this.git.reset(["HEAD", "."]) // git reset HEAD .
161-
await this.git.clean([CleanOptions.FORCE, CleanOptions.RECURSIVE]) // git clean -f -d
162-
await this.git.reset(["--hard", latestHash]) // git reset --hard <latestHash>
163-
await this.git.checkout(this.mainBranch) // git checkout <mainBranch>
164-
await this.popStash() // git stash pop
153+
await reset()
165154
return undefined
166155
}
167156

@@ -173,13 +162,18 @@ export class CheckpointService {
173162
await this.popStash()
174163
return commit
175164
} catch (err) {
176-
// Ensure we return to main branch and pop stash even if something
177-
// fails.
178-
// @TODO: Disable checkpoints since we encountered an error.
179165
this.log(`[saveCheckpoint] Failed to save checkpoint: ${err instanceof Error ? err.message : String(err)}`)
180-
await this.git.reset(["HEAD", "."]) // git reset HEAD .
181-
await this.git.checkout(this.mainBranch) // git checkout <mainBranch>
182-
await this.popStash() // git stash pop
166+
167+
// If we're not on the main branch, we need to trigger a reset
168+
// (equivalent to the empty diff case above).
169+
// This ensures that we return to the main branch and restore the
170+
// pending changes.
171+
const currentBranch = await this.git.revparse(["--abbrev-ref", "HEAD"])
172+
173+
if (currentBranch.trim() !== this.mainBranch) {
174+
await reset()
175+
}
176+
183177
throw err
184178
}
185179
}

src/services/checkpoints/__tests__/CheckpointService.test.ts

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -167,20 +167,6 @@ describe("CheckpointService", () => {
167167
)
168168
})
169169

170-
it("handles stash failures gracefully", async () => {
171-
await fs.writeFile(testFile, "Stash failure incoming!")
172-
173-
// Mock git stash to simulate failure.
174-
jest.spyOn(git, "stash").mockRejectedValue(new Error("Simulated stash failure"))
175-
176-
// Attempt to save checkpoint.
177-
const commit = await service.saveCheckpoint("test")
178-
expect(commit).toBeUndefined()
179-
180-
// Verify working directory is unchanged.
181-
expect(await fs.readFile(testFile, "utf-8")).toEqual("Stash failure incoming!")
182-
})
183-
184170
it("cleans up staged files if a commit fails", async () => {
185171
await fs.writeFile(testFile, "Changed content")
186172

0 commit comments

Comments
 (0)