Skip to content

Commit

Permalink
feat: Handle CronJob not found
Browse files Browse the repository at this point in the history
Before this patch, if the Renovate CronJob could not be found, we
would return a 500 Internal Server Error and the UI would look
broken. Now, we return a 404 Not Found, log a warning, and let the
frontend show an error message component. The info cards are
updated to properly handle this case.
  • Loading branch information
meyfa committed Jul 13, 2024
1 parent e2e5b1a commit fdffa60
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 28 deletions.
17 changes: 11 additions & 6 deletions backend/src/api/cronjob.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ export interface CronJobRoute {
Reply: {
schedule?: string
timeZone?: string
nextScheduleTime?: string
suspend: boolean
nextScheduleTime: string
}
}

Expand All @@ -27,15 +27,20 @@ export const cronjobRoute = ({ cronJobController }: Controllers): FastifyPluginA
}

const schedule = typeof cronJob.spec?.schedule === 'string' ? cronJob.spec.schedule : undefined
const interval = cronParser.parseExpression(schedule ?? '', {
tz: typeof cronJob.spec?.timeZone === 'string' ? cronJob.spec.timeZone : undefined
})

let nextScheduleTime: string | undefined
if (schedule != null) {
const interval = cronParser.parseExpression(schedule ?? '', {
tz: typeof cronJob.spec?.timeZone === 'string' ? cronJob.spec.timeZone : undefined
})
nextScheduleTime = interval.next().toISOString()
}

return {
schedule,
timeZone: cronJob.spec?.timeZone,
suspend: cronJob.spec?.suspend ?? false,
nextScheduleTime: interval.next().toISOString()
nextScheduleTime,
suspend: cronJob.spec?.suspend ?? false
}
})
}
14 changes: 11 additions & 3 deletions backend/src/kubernetes/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,18 @@ export class KubernetesApi {
name: string
}): Promise<V1CronJob | undefined> {
this.log.debug({ options }, 'k8s_getCronJob')
const result = await this.request(async () => {
return await this.batchApi.readNamespacedCronJob(options.name, options.namespace)
return await this.request(async () => {
try {
const result = await this.batchApi.readNamespacedCronJob(options.name, options.namespace)
return result.body
} catch (err) {
if (err instanceof HttpError && err.statusCode === 404) {
this.log.warn({ options }, 'k8s_getCronJob: not found')
return undefined
}
throw err
}
})
return result.body
}

async getJobs (options: {
Expand Down
7 changes: 3 additions & 4 deletions frontend/src/components/InfoCard.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import { FunctionComponent, PropsWithChildren } from 'react'
import { ColoredSkeleton } from './ColoredSkeleton.js'
import { Card } from './Card.js'

export const InfoCard: FunctionComponent<PropsWithChildren<{
title?: string
title: string
}>> = (props) => {
return (
<Card>
<div className='text-sm text-gray-200'>{props.title ?? <ColoredSkeleton />}</div>
<div className='mt-2 text-white-900 font-semibold'>{props.children ?? <ColoredSkeleton />}</div>
<div className='text-sm text-gray-200'>{props.title}</div>
<div className='mt-2 text-white-900 font-semibold'>{props.children}</div>
</Card>
)
}
2 changes: 1 addition & 1 deletion frontend/src/components/Navigation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const NavigationJobsSection: FunctionComponent<{
{!loading
? (
<NavigationItem to='/jobs' onNavigate={props.onNavigate}>
View all jobs ({jobs?.length})
View all jobs {jobs != null ? `(${jobs.length})` : ''}
</NavigationItem>
)
: (
Expand Down
40 changes: 26 additions & 14 deletions frontend/src/pages/Home.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { JobPanel } from '../components/JobPanel.js'
import { ColoredSkeleton } from '../components/ColoredSkeleton.js'
import { Link } from 'react-router-dom'
import { useMobileNavigation } from '../util/navigation.js'
import { ErrorMessage } from '../components/ErrorMessage.js'

const JOB_HISTORY_LIMIT = 3

Expand All @@ -18,7 +19,7 @@ function lowercaseFirstLetter (str: string): string {
}

export const Home: FunctionComponent = () => {
const { loading, data: cronJob } = useApiSubscription({ interval: 60_000 }, api.cronJob)
const { data: cronJob, error } = useApiSubscription({ interval: 60_000 }, api.cronJob)

const isMobileNavigation = useMobileNavigation()

Expand All @@ -27,23 +28,34 @@ export const Home: FunctionComponent = () => {
<Heading>
Overview
</Heading>
<dl className='my-1 grid grid-cols-1 md:grid-cols-2 gap-4'>
{error != null && (
<ErrorMessage>
Error loading CronJob: {error.message}
</ErrorMessage>
)}
<div className='my-1 grid grid-cols-1 md:grid-cols-2 gap-4'>
<InfoCard title='Next run'>
{!loading && cronJob != null
? (
<>
{DateTime.fromISO(cronJob.nextScheduleTime).toLocaleString(DateTime.DATETIME_MED)}&nbsp;
({DateTime.fromISO(cronJob.nextScheduleTime).toRelative()})
</>
)
: null}
{cronJob == null && <ColoredSkeleton />}
{cronJob?.nextScheduleTime != null && (
<>
{DateTime.fromISO(cronJob.nextScheduleTime).toLocaleString(DateTime.DATETIME_MED)}&nbsp;
({DateTime.fromISO(cronJob.nextScheduleTime).toRelative()})
</>
)}
{cronJob != null && cronJob.nextScheduleTime == null && (
'Not scheduled'
)}
</InfoCard>
<InfoCard title='CronJob schedule'>
{!loading && cronJob?.schedule != null
? `${cronJob.schedule} (${lowercaseFirstLetter(cronstrue.toString((cronJob.schedule)))})`
: null}
{cronJob == null && <ColoredSkeleton />}
{cronJob?.schedule != null && (
`${cronJob.schedule} (${lowercaseFirstLetter(cronstrue.toString((cronJob.schedule)))})`
)}
{cronJob != null && cronJob.schedule == null && (
'Not scheduled'
)}
</InfoCard>
</dl>
</div>
{isMobileNavigation && <JobsSection />}
</>
)
Expand Down

0 comments on commit fdffa60

Please sign in to comment.