Skip to content

Commit 3b53e6b

Browse files
CasLubbersotomi-adminsvcAPLBot
authored
feat: apl-operator clean error messages (#2290)
Co-authored-by: otomi-admin <[email protected]> Co-authored-by: svcAPLBot <[email protected]>
1 parent 41560c1 commit 3b53e6b

File tree

8 files changed

+60
-41
lines changed

8 files changed

+60
-41
lines changed

‎src/operator/apl-operations.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { module as bootstrapModule } from '../cmd/bootstrap'
66
import { module as validateValuesModule } from '../cmd/validate-values'
77
import { module as migrateModule } from '../cmd/migrate'
88
import { OperatorError } from './errors'
9+
import { getErrorMessage } from './utils'
910

1011
export class AplOperations {
1112
private d: OtomiDebugger
@@ -26,7 +27,7 @@ export class AplOperations {
2627
await migrateModule.handler(args)
2728
this.d.info('Migration completed successfully')
2829
} catch (error) {
29-
this.d.error('Migration failed:', error)
30+
this.d.error('Migration failed:', getErrorMessage(error))
3031
throw new OperatorError('Migration process failed', error as Error)
3132
}
3233
}
@@ -38,7 +39,7 @@ export class AplOperations {
3839
await bootstrapModule.handler({} as HelmArguments)
3940
this.d.info('Bootstrap completed successfully')
4041
} catch (error) {
41-
this.d.error('Bootstrap failed:', error)
42+
this.d.error('Bootstrap failed:', getErrorMessage(error))
4243
throw new OperatorError('Bootstrap process failed', error as Error)
4344
}
4445
}
@@ -50,7 +51,7 @@ export class AplOperations {
5051
await validateValuesModule.handler({} as HelmArguments)
5152
this.d.info('Values validation completed successfully')
5253
} catch (error) {
53-
this.d.error('Values validation failed:', error)
54+
this.d.error('Values validation failed:', getErrorMessage(error))
5455
throw new OperatorError('Values validation failed', error as Error)
5556
}
5657
}
@@ -68,7 +69,7 @@ export class AplOperations {
6869
await applyModule.handler(args)
6970
this.d.info('Apply completed successfully')
7071
} catch (error) {
71-
this.d.error('Apply failed:', error)
72+
this.d.error('Apply failed:', getErrorMessage(error))
7273
throw new OperatorError('Apply operation failed', error as Error)
7374
}
7475
}
@@ -84,7 +85,7 @@ export class AplOperations {
8485
await applyAsAppsModule.handler(args)
8586
this.d.info('ApplyAsApps for teams completed successfully')
8687
} catch (error) {
87-
this.d.error('ApplyAsApps for teams failed:', error)
88+
this.d.error('ApplyAsApps for teams failed:', getErrorMessage(error))
8889
throw new OperatorError('ApplyAsApps for teams failed', error as Error)
8990
}
9091
}

‎src/operator/apl-operator.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ describe('AplOperator', () => {
123123
expect(waitTillGitRepoAvailable).toHaveBeenCalledWith(mockGitRepo.repoUrl)
124124
expect(mockGitRepo.clone).toHaveBeenCalled()
125125

126-
expect(mockErrorFn).toHaveBeenCalledWith('Failed to start APL operator:', error)
126+
expect(mockErrorFn).toHaveBeenCalledWith('Failed to start APL operator:', 'Start failed')
127127
})
128128

129129
test('should log error if poll or reconcile fails after start', async () => {
@@ -142,7 +142,7 @@ describe('AplOperator', () => {
142142

143143
await aplOperator.start()
144144

145-
expect(mockErrorFn).toHaveBeenCalledWith('Error in polling or reconcile task:', expect.any(Error))
145+
expect(mockErrorFn).toHaveBeenCalledWith('Error in polling or reconcile task:', 'Reconcile crashed')
146146
})
147147

148148
test('should handle already running state', async () => {
@@ -255,7 +255,7 @@ describe('AplOperator', () => {
255255

256256
expect((aplOperator as any).isApplying).toBe(false)
257257

258-
expect(mockErrorFn).toHaveBeenCalledWith('[poll] Apply process failed', error)
258+
expect(mockErrorFn).toHaveBeenCalledWith('[poll] Apply process failed', 'Apply failed')
259259
})
260260
})
261261

@@ -339,7 +339,7 @@ describe('AplOperator', () => {
339339

340340
await pollPromise
341341

342-
expect(mockErrorFn).toHaveBeenCalledWith('Error during git polling cycle:', expect.any(Error))
342+
expect(mockErrorFn).toHaveBeenCalledWith('Error during git polling cycle:', 'Pull failed')
343343
})
344344
})
345345

@@ -390,7 +390,7 @@ describe('AplOperator', () => {
390390
await reconcilePromise
391391

392392
expect(runApplyIfNotBusySpy).toHaveBeenCalled()
393-
expect(mockErrorFn).toHaveBeenCalledWith('Error during reconciliation:', error)
393+
expect(mockErrorFn).toHaveBeenCalledWith('Error during reconciliation:', 'Reconcile error')
394394
expect(mockInfoFn).toHaveBeenCalledWith('Reconciliation loop stopped')
395395
})
396396
})

‎src/operator/apl-operator.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { commit } from '../cmd/commit'
99
import { HelmArguments } from '../common/yargs'
1010
import { hfValues } from '../common/hf'
1111
import { writeValues } from '../common/values'
12+
import { getErrorMessage } from './utils'
1213

1314
export interface AplOperatorConfig {
1415
gitRepo: GitRepository
@@ -97,13 +98,14 @@ export class AplOperator {
9798
trigger,
9899
})
99100
} catch (error) {
100-
this.d.error(`[${trigger}] Apply process failed`, error)
101+
const errorMessage = getErrorMessage(error)
102+
this.d.error(`[${trigger}] Apply process failed`, errorMessage)
101103
await updateApplyState({
102104
commitHash,
103105
status: 'failed',
104106
timestamp: new Date().toISOString(),
105107
trigger,
106-
errorMessage: error instanceof Error ? error.message : String(error),
108+
errorMessage,
107109
})
108110
} finally {
109111
this.isApplying = false
@@ -120,7 +122,7 @@ export class AplOperator {
120122
await this.runApplyIfNotBusy(ApplyTrigger.Reconcile)
121123
this.d.info('Reconciliation completed')
122124
} catch (error) {
123-
this.d.error('Error during reconciliation:', error)
125+
this.d.error('Error during reconciliation:', getErrorMessage(error))
124126
}
125127

126128
await this.scheduleNextAttempt(this.reconcileInterval)
@@ -147,7 +149,7 @@ export class AplOperator {
147149
await this.runApplyIfNotBusy(ApplyTrigger.Poll, applyTeamsOnly)
148150
}
149151
} catch (error) {
150-
this.d.error('Error during git polling cycle:', error)
152+
this.d.error('Error during git polling cycle:', getErrorMessage(error))
151153
}
152154

153155
await this.scheduleNextAttempt(this.pollInterval)
@@ -181,14 +183,14 @@ export class AplOperator {
181183
this.d.info('APL operator started successfully')
182184
} catch (error) {
183185
this.isRunning = false
184-
this.d.error('Failed to start APL operator:', error)
186+
this.d.error('Failed to start APL operator:', getErrorMessage(error))
185187
throw error
186188
}
187189

188190
try {
189191
await Promise.all([this.pollAndApplyGitChanges(), this.reconcile()])
190192
} catch (error) {
191-
this.d.error('Error in polling or reconcile task:', error)
193+
this.d.error('Error in polling or reconcile task:', getErrorMessage(error))
192194
}
193195
}
194196

‎src/operator/git-repository.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,14 @@ describe('GitRepository', () => {
8888

8989
describe('waitForCommits', () => {
9090
test('should retry until hasCommits returns true', async () => {
91-
const hasCommitsSpy = jest.spyOn(gitRepository, 'setLastRevision')
92-
93-
hasCommitsSpy.mockRejectedValueOnce(new Error('No commits yet')).mockResolvedValueOnce()
91+
const setLastRevisionSpy = jest.spyOn(gitRepository, 'setLastRevision')
92+
mockGit.pull.mockResolvedValue({})
93+
setLastRevisionSpy.mockRejectedValueOnce(new Error('No commits yet')).mockResolvedValueOnce()
9494

9595
await gitRepository.waitForCommits(2, 100)
9696

97-
expect(hasCommitsSpy).toHaveBeenCalledTimes(2)
97+
expect(mockGit.pull).toHaveBeenCalledTimes(2)
98+
expect(setLastRevisionSpy).toHaveBeenCalledTimes(2)
9899
})
99100
})
100101

‎src/operator/git-repository.ts

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import simpleGit, { SimpleGit } from 'simple-git'
22
import { OtomiDebugger, terminal } from '../common/debug'
3-
import retry, { Options } from 'async-retry'
3+
import retry from 'async-retry'
44
import { OperatorError } from './errors'
5+
import { getErrorMessage } from './utils'
6+
import { env } from '../common/envalid'
57

68
export interface GitRepositoryConfig {
79
username: string
@@ -38,27 +40,27 @@ export class GitRepository {
3840
this._lastRevision = logs.latest?.hash || ''
3941
}
4042
} catch (error) {
41-
this.d.warn('Gitea has no commits yet:', error)
43+
this.d.warn('Gitea has no commits yet:', getErrorMessage(error))
4244
throw error
4345
}
4446
}
4547

46-
async waitForCommits(maxRetries = 30, interval = 10000): Promise<void> {
48+
async waitForCommits(maxRetries = env.RETRIES, interval = env.MIN_TIMEOUT): Promise<void> {
4749
this.d.info(`Waiting for repository to have commits (max ${maxRetries} retries, ${interval}ms interval)`)
4850

49-
const retryOptions: Options = {
50-
retries: 20,
51-
maxTimeout: 30000,
52-
}
5351
const d = terminal('common:k8s:waitTillGitRepoAvailable')
54-
await retry(async () => {
55-
try {
56-
await this.setLastRevision()
57-
} catch (e) {
58-
d.warn(`The values repository has no commits yet. Retrying in ${retryOptions.maxTimeout} ms`)
59-
throw e
60-
}
61-
}, retryOptions)
52+
await retry(
53+
async () => {
54+
try {
55+
await this.git.pull()
56+
await this.setLastRevision()
57+
} catch (e) {
58+
d.warn(`The values repository has no commits yet. Retrying in ${interval} ms`)
59+
throw e
60+
}
61+
},
62+
{ retries: maxRetries, randomize: env.RANDOM, minTimeout: interval, factor: env.FACTOR },
63+
)
6264
}
6365

6466
async clone(): Promise<void> {
@@ -68,7 +70,7 @@ export class GitRepository {
6870
await this.git.clone(this.repoUrl, this.repoPath)
6971
this.d.info(`Repository cloned successfully`)
7072
} catch (error) {
71-
this.d.error('Failed to clone repository:', error)
73+
this.d.error('Failed to clone repository:', getErrorMessage(error))
7274
throw new OperatorError('Repository clone failed', error as Error)
7375
}
7476
}
@@ -99,7 +101,7 @@ export class GitRepository {
99101
await this.git.pull()
100102
return this.getCurrentRevision()
101103
} catch (error) {
102-
this.d.error('Failed to pull repository:', error)
104+
this.d.error('Failed to pull repository:', getErrorMessage(error))
103105
throw new OperatorError('Repository pull failed', error as Error)
104106
}
105107
}
@@ -152,7 +154,7 @@ export class GitRepository {
152154
applyTeamsOnly: onlyTeamsChanged,
153155
}
154156
} catch (error) {
155-
this.d.error('Failed to analyze repository changes:', error)
157+
this.d.error('Failed to analyze repository changes:', getErrorMessage(error))
156158
throw new OperatorError('Repository sync and analysis failed', error as Error)
157159
}
158160
}

‎src/operator/k8s.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { terminal } from '../common/debug'
22
import { ApiException, CoreV1Api, KubeConfig } from '@kubernetes/client-node'
3+
import { getErrorMessage } from './utils'
34

45
export type ApplyStatus = 'succeeded' | 'failed' | 'in-progress' | 'unknown'
56

@@ -71,6 +72,6 @@ export async function updateApplyState(
7172

7273
d.info(`Apply state updated successfully for commit ${state.commitHash}`)
7374
} catch (error) {
74-
d.error('Failed to update apply state:', error)
75+
d.error('Failed to update apply state:', getErrorMessage(error))
7576
}
7677
}

‎src/operator/main.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import fs from 'fs'
77
import path from 'path'
88
import { GitRepository } from './git-repository'
99
import { AplOperations } from './apl-operations'
10+
import { getErrorMessage } from './utils'
1011

1112
dotenv.config()
1213

@@ -83,14 +84,14 @@ async function main(): Promise<void> {
8384

8485
await operator.start()
8586
} catch (error) {
86-
d.error('Failed to start APL Operator:', error)
87+
d.error('Failed to start APL Operator:', getErrorMessage(error))
8788
process.exit(1)
8889
}
8990
}
9091

9192
if (require.main === module) {
9293
main().catch((error) => {
93-
d.error('Unhandled error in main:', error)
94+
d.error('Unhandled error in main:', getErrorMessage(error))
9495
process.exit(1)
9596
})
9697
}

‎src/operator/utils.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
export function getErrorMessage(error: unknown): string {
2+
if (error instanceof Error) {
3+
return error.message
4+
}
5+
6+
if (typeof error === 'string') {
7+
return error
8+
}
9+
10+
return String(error)
11+
}

0 commit comments

Comments
 (0)