Skip to content

Commit

Permalink
Merge pull request #48 from flying-dice/fix/catch-errors-on-failed-tasks
Browse files Browse the repository at this point in the history
Fix/catch errors on failed tasks
  • Loading branch information
JonathanTurnock authored Dec 15, 2024
2 parents 2c6f079 + 60011cc commit dbdd627
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 31 deletions.
4 changes: 3 additions & 1 deletion src/main/manager/subscription.manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export type SubscriptionReleaseState = {
isLatest: boolean
latest?: string
isReady: boolean
isFailed: boolean
}

export type SubscriptionWithState = {
Expand Down Expand Up @@ -115,7 +116,8 @@ export class SubscriptionManager implements OnApplicationBootstrap {
isReady: taskStatuses.every((it) => it === AssetTaskStatus.COMPLETED),
exePath: installed.exePath,
isLatest: installed.version === latest?.version || true,
latest: latest?.version
latest: latest?.version,
isFailed: taskStatuses.some((it) => it === AssetTaskStatus.FAILED)
}
}

Expand Down
27 changes: 20 additions & 7 deletions src/main/manager/task.manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { ReleaseService } from '../services/release.service'
import { SubscriptionService } from '../services/subscription.service'
import { InjectConnection } from '@nestjs/mongoose'
import { Connection, ConnectionStates } from 'mongoose'
import { findFirstPendingTask } from '../utils/find-first-pending-task'

@Injectable()
export class TaskManager implements OnApplicationBootstrap, OnApplicationShutdown {
Expand Down Expand Up @@ -61,19 +62,31 @@ export class TaskManager implements OnApplicationBootstrap, OnApplicationShutdow
}

async checkForPendingTasks() {
// Get the current task being processed
let task = await this.releaseService.findInProgressAssetTask()

// If there is no task being processed, get the next pending task by going in ID descending order sequentially
if (!task) {
task = await this.releaseService.findPendingAssetTaskSortedBySequence()
}
for (const subscription of await this.subscriptionService.findAll()) {
const release = await this.releaseService.findBySubscriptionIdOrThrow(subscription.id)
const assets = await this.releaseService.findAssetsByRelease(release.id)

for (const asset of assets) {
const tasks = await this.releaseService.findAssetTasksByAssetId(asset.id)
const firstPendingTask = findFirstPendingTask(tasks)

if (firstPendingTask) {
task = firstPendingTask
break
}
}

// If there is still no task to process, log a message and return as this means there are no tasks to process in the database
if (!task) {
return
if (task) {
break
}
}
}

if (!task) return

// Get the processor for the task to process or create a new one if it doesn't exist
// Use cache to avoid creating a new processor for the same task multiple times and allow internal state to be maintained
const processor =
Expand Down
16 changes: 8 additions & 8 deletions src/main/services/release.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,17 +135,17 @@ export class ReleaseService {
.then((it) => it?.toJSON())
}

async findPendingAssetTaskSortedBySequence(): Promise<AssetTask | undefined> {
return this.assetTasks
.findOne({ status: AssetTaskStatus.PENDING })
.sort({ sequence: 1, createdAt: 1 })
.exec()
.then((it) => it?.toJSON())
}

async deleteBySubscriptionId(id: string) {
await this.assetTasks.deleteMany({ subscriptionId: id }).exec()
await this.releaseAssets.deleteMany({ subscriptionId: id }).exec()
await this.releases.deleteOne({ subscriptionId: id }).exec()
}

async findAssetTasksByAssetId(id: string): Promise<AssetTask[]> {
return this.assetTasks
.find({ releaseAssetId: id })
.sort({ sequence: 1 })
.exec()
.then((it) => it.map((it) => it.toJSON()))
}
}
13 changes: 0 additions & 13 deletions src/main/services/subscription.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,23 @@ import { Injectable } from '@nestjs/common'
import { InjectModel } from '@nestjs/mongoose'
import { Subscription } from '../schemas/subscription.schema'
import { Model } from 'mongoose'
import { Log } from '../utils/log'

@Injectable()
export class SubscriptionService {
@InjectModel(Subscription.name)
private readonly subscriptions: Model<Subscription>

@Log()
async deleteById(id: string): Promise<void> {
await this.subscriptions.deleteOne({ id }).exec()
}

@Log()
async findById(id: string): Promise<Subscription | undefined> {
return this.subscriptions
.findOne({ id })
.exec()
.then((it) => it?.toJSON() ?? undefined)
}

@Log()
async findAll(): Promise<Subscription[]> {
return this.subscriptions
.find()
Expand All @@ -31,7 +27,6 @@ export class SubscriptionService {
.then((it) => it.map((it) => it.toJSON()))
}

@Log()
async findByIdOrThrow(id: string): Promise<Subscription> {
return this.subscriptions
.findOne({ id })
Expand All @@ -40,15 +35,13 @@ export class SubscriptionService {
.then((it) => it?.toJSON() ?? undefined)
}

@Log()
async findByModId(modId: string): Promise<Subscription | undefined> {
return this.subscriptions
.findOne({ modId })
.exec()
.then((it) => it?.toJSON() ?? undefined)
}

@Log()
async findByModIdOrThrow(modId: string): Promise<Subscription> {
return this.subscriptions
.findOne({ modId })
Expand All @@ -57,12 +50,6 @@ export class SubscriptionService {
.then((it) => it.toJSON())
}

@Log()
async deleteByModId(modId: string): Promise<void> {
await this.subscriptions.deleteOne({ modId }).exec()
}

@Log()
async save(subscription: Subscription): Promise<Subscription> {
await this.subscriptions
.updateOne({ modId: subscription.modId }, subscription, { upsert: true })
Expand Down
47 changes: 47 additions & 0 deletions src/main/utils/find-first-pending-task.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { describe, expect, it } from 'vitest'
import { findFirstPendingTask } from './find-first-pending-task'
import { AssetTaskStatus, AssetTaskType } from '../schemas/release-asset-task.schema'

describe('findFirstPending', () => {
it('should return the first pending task with sequence 1', () => {
const tasks = [
{ id: '1', sequence: 1, status: AssetTaskStatus.PENDING, type: AssetTaskType.DOWNLOAD },
{ id: '2', sequence: 2, status: AssetTaskStatus.PENDING, type: AssetTaskType.EXTRACT }
]
const result = findFirstPendingTask(tasks)
expect(result).toEqual(tasks[0])
})

it('should return the first pending task with sequence greater than 1 if previous task is completed', () => {
const tasks = [
{ id: '1', sequence: 1, status: AssetTaskStatus.COMPLETED, type: AssetTaskType.DOWNLOAD },
{ id: '2', sequence: 2, status: AssetTaskStatus.PENDING, type: AssetTaskType.EXTRACT }
]
const result = findFirstPendingTask(tasks)
expect(result).toEqual(tasks[1])
})

it('should return undefined if no tasks are pending', () => {
const tasks = [
{ id: '1', sequence: 1, status: AssetTaskStatus.COMPLETED, type: AssetTaskType.DOWNLOAD },
{ id: '2', sequence: 2, status: AssetTaskStatus.COMPLETED, type: AssetTaskType.EXTRACT }
]
const result = findFirstPendingTask(tasks)
expect(result).toBeUndefined()
})

it('should return undefined if no tasks exist', () => {
const tasks = []
const result = findFirstPendingTask(tasks)
expect(result).toBeUndefined()
})

it('should return undefined if the first task is not pending and no previous task is completed', () => {
const tasks = [
{ id: '1', sequence: 1, status: AssetTaskStatus.FAILED, type: AssetTaskType.DOWNLOAD },
{ id: '2', sequence: 2, status: AssetTaskStatus.PENDING, type: AssetTaskType.EXTRACT }
]
const result = findFirstPendingTask(tasks)
expect(result).toBeUndefined()
})
})
21 changes: 21 additions & 0 deletions src/main/utils/find-first-pending-task.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { AssetTask } from '../schemas/release-asset-task.schema'

export function findFirstPendingTask<T extends Pick<AssetTask, 'status' | 'sequence'>>(
tasks: T[]
): T | undefined {
let previousTask: T | undefined

for (const task of tasks) {
if (task.status === 'PENDING') {
if (previousTask?.status === 'COMPLETED') {
return task
}
if (!previousTask) {
return task
}
}
previousTask = task
}

return undefined
}
3 changes: 2 additions & 1 deletion src/renderer/src/components/subscription-row.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export type SubscriptionRowProps = {
onRunExe?: () => void
onUnsubscribe: () => void
isReady: boolean
isFailed: boolean
stateLabel: string
progress: number
}
Expand Down Expand Up @@ -62,7 +63,7 @@ export function SubscriptionRow(props: SubscriptionRowProps) {
</Group>
</Table.Td>
<Table.Td>
{!props.isReady ? (
{!props.isReady && !props.isFailed ? (
<Tooltip label={props.stateLabel}>
<Progress.Root size="lg">
<Progress.Section value={props.progress} striped animated>
Expand Down
3 changes: 2 additions & 1 deletion src/renderer/src/container/subscriptions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export function Subscriptions({ onOpenSymlinksModal }: SubscriptionsProps) {
const { results, search, setSearch } = useFuse(subscriptions.data || [], '', ['modId', 'modName'])

useEffect(() => {
if (subscriptions.data?.some(({ state }) => state.progress !== 100)) {
if (subscriptions.data?.some(({ state }) => state.progress !== 100 && !state.isFailed)) {
setTimeout(() => subscriptions.mutate(), 500)
}
}, [subscriptions.data])
Expand Down Expand Up @@ -110,6 +110,7 @@ export function Subscriptions({ onOpenSymlinksModal }: SubscriptionsProps) {
? () => state.exePath && handleRunExe(subscription.modId, state.exePath)
: undefined
}
isFailed={state.isFailed}
/>
))}
</Table.Tbody>
Expand Down

0 comments on commit dbdd627

Please sign in to comment.