Skip to content

Commit f6a9f37

Browse files
nytaminnytamin
committed
fix: add knownReason to the accessorHandle/expectationHandler method responses.
This is to indicate that the reason for an unsuccessful result might be of an issue in the worker that might be solved by a restart of the worker.
1 parent d7d1618 commit f6a9f37

32 files changed

+624
-132
lines changed

‎shared/packages/api/src/worker.ts

+18
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export type ReturnTypeDoYouSupportExpectation =
1212
| {
1313
support: false
1414
reason: Reason
15+
knownReason: KnownReason
1516
}
1617
export type ReturnTypeGetCostFortExpectation = {
1718
/** (null means "infinite cost") */
@@ -32,6 +33,7 @@ export type ReturnTypeIsExpectationReadyToStartWorkingOn =
3233
sourceExists?: boolean
3334
isPlaceholder?: boolean
3435
reason: Reason
36+
knownReason: KnownReason
3537
}
3638
export type ReturnTypeIsExpectationFulfilled =
3739
| {
@@ -40,6 +42,7 @@ export type ReturnTypeIsExpectationFulfilled =
4042
| {
4143
fulfilled: false
4244
reason: Reason
45+
knownReason: KnownReason
4346
}
4447
export type ReturnTypeRemoveExpectation =
4548
| {
@@ -48,6 +51,7 @@ export type ReturnTypeRemoveExpectation =
4851
| {
4952
removed: false
5053
reason: Reason
54+
knownReason: KnownReason
5155
}
5256

5357
/** Configurations for any of the workers */
@@ -74,6 +78,7 @@ export type ReturnTypeDoYouSupportPackageContainer =
7478
| {
7579
support: false
7680
reason: Reason
81+
knownReason: KnownReason
7782
}
7883
export type ReturnTypeRunPackageContainerCronJob =
7984
| {
@@ -82,6 +87,7 @@ export type ReturnTypeRunPackageContainerCronJob =
8287
| {
8388
success: false
8489
reason: Reason
90+
knownReason: KnownReason
8591
}
8692
export type ReturnTypeDisposePackageContainerMonitors =
8793
| {
@@ -90,6 +96,7 @@ export type ReturnTypeDisposePackageContainerMonitors =
9096
| {
9197
success: false
9298
reason: Reason
99+
knownReason: KnownReason
93100
}
94101
export type ReturnTypeSetupPackageContainerMonitors =
95102
| {
@@ -99,6 +106,7 @@ export type ReturnTypeSetupPackageContainerMonitors =
99106
| {
100107
success: false
101108
reason: Reason
109+
knownReason: KnownReason
102110
}
103111
export interface MonitorProperties {
104112
label: string
@@ -110,3 +118,13 @@ export type Cost = number | null // Note: we're using null to represent infinity
110118
export function valueOfCost(cost: Cost): number {
111119
return cost === null ? Number.POSITIVE_INFINITY : cost
112120
}
121+
122+
/**
123+
* This represents a flag that indicates if the reason for being unsuccessful is well known.
124+
* - It should be set to true if the reason for being unsuccessful is well known.
125+
* - If should be set to if there is a chance that the error has an unknown/external origin.
126+
* If this happens enough times, a worker might eventually be restarted to try to solve the issue
127+
* @see config.worker.failurePeriod.
128+
* @see config.worker.failurePeriodLimit.
129+
* */
130+
export type KnownReason = boolean

‎shared/packages/worker/src/worker/accessorHandlers/atem.ts

+7
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ export class ATEMAccessorHandle<Metadata> extends GenericAccessorHandle<Metadata
7777
if (this.accessor.type !== Accessor.AccessType.ATEM_MEDIA_STORE) {
7878
return {
7979
success: false,
80+
knownReason: false,
8081
reason: {
8182
user: `There is an internal issue in Package Manager`,
8283
tech: `ATEM Accessor type is not ATEM ("${this.accessor.type}")!`,
@@ -86,6 +87,7 @@ export class ATEMAccessorHandle<Metadata> extends GenericAccessorHandle<Metadata
8687
if (!this.accessor.mediaType) {
8788
return {
8889
success: false,
90+
knownReason: true,
8991
reason: {
9092
user: `Accessor mediaType not set`,
9193
tech: `Accessor mediaType not set`,
@@ -95,6 +97,7 @@ export class ATEMAccessorHandle<Metadata> extends GenericAccessorHandle<Metadata
9597
if (typeof this.accessor.bankIndex !== 'number') {
9698
return {
9799
success: false,
100+
knownReason: true,
98101
reason: {
99102
user: `Accessor bankIndex not set`,
100103
tech: `Accessor bankIndex not set`,
@@ -141,6 +144,7 @@ export class ATEMAccessorHandle<Metadata> extends GenericAccessorHandle<Metadata
141144
// Reading actually not supprted, but oh well..
142145
return {
143146
success: false,
147+
knownReason: true,
144148
reason: {
145149
user: 'Clip not found',
146150
tech: `Clip "${this.getAtemClipName()}" not found`,
@@ -152,6 +156,7 @@ export class ATEMAccessorHandle<Metadata> extends GenericAccessorHandle<Metadata
152156
if (!atem.state?.media.clipPool || !atem.state?.media.stillPool) {
153157
return {
154158
success: false,
159+
knownReason: true,
155160
packageExists: false,
156161
reason: {
157162
user: `ATEM media pools are inaccessible`,
@@ -177,6 +182,7 @@ export class ATEMAccessorHandle<Metadata> extends GenericAccessorHandle<Metadata
177182
const errorHandler = (error: string) => {
178183
resolve({
179184
success: false,
185+
knownReason: false,
180186
reason: {
181187
user: `Error connecting to ATEM`,
182188
tech: error,
@@ -447,6 +453,7 @@ export class ATEMAccessorHandle<Metadata> extends GenericAccessorHandle<Metadata
447453
async setupPackageContainerMonitors(): Promise<SetupPackageContainerMonitorsResult> {
448454
return {
449455
success: false,
456+
knownReason: false,
450457
reason: {
451458
user: `There is an internal issue in Package Manager`,
452459
tech: 'setupPackageContainerMonitors, not supported',

‎shared/packages/worker/src/worker/accessorHandlers/corePackageInfo.ts

+2
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ export class CorePackageInfoAccessorHandle<Metadata> extends GenericAccessorHand
6262
if (this.accessor.type !== Accessor.AccessType.CORE_PACKAGE_INFO) {
6363
return {
6464
success: false,
65+
knownReason: false,
6566
reason: {
6667
user: `There is an internal issue in Package Manager`,
6768
tech: `CorePackageInfo Accessor type is not CORE_PACKAGE_INFO ("${this.accessor.type}")!`,
@@ -146,6 +147,7 @@ export class CorePackageInfoAccessorHandle<Metadata> extends GenericAccessorHand
146147
async setupPackageContainerMonitors(): Promise<SetupPackageContainerMonitorsResult> {
147148
return {
148149
success: false,
150+
knownReason: false,
149151
reason: {
150152
user: `There is an internal issue in Package Manager`,
151153
tech: 'setupPackageContainerMonitors, not supported',

‎shared/packages/worker/src/worker/accessorHandlers/fileShare.ts

+20-3
Original file line numberDiff line numberDiff line change
@@ -123,22 +123,32 @@ export class FileShareAccessorHandle<Metadata> extends GenericFileAccessorHandle
123123
if (this.accessor.type !== Accessor.AccessType.FILE_SHARE) {
124124
return {
125125
success: false,
126+
knownReason: false,
126127
reason: {
127128
user: `There is an internal issue in Package Manager`,
128129
tech: `FileShare Accessor type is not FILE_SHARE ("${this.accessor.type}")!`,
129130
},
130131
}
131132
}
132133
if (!this.originalFolderPath)
133-
return { success: false, reason: { user: `Folder path not set`, tech: `Folder path not set` } }
134+
return {
135+
success: false,
136+
knownReason: true,
137+
reason: { user: `Folder path not set`, tech: `Folder path not set` },
138+
}
134139
if (!this.content.onlyContainerAccess) {
135140
if (!this.filePath)
136-
return { success: false, reason: { user: `File path not set`, tech: `File path not set` } }
141+
return {
142+
success: false,
143+
knownReason: true,
144+
reason: { user: `File path not set`, tech: `File path not set` },
145+
}
137146

138147
// Don't allow absolute file paths:
139148
if (betterPathIsAbsolute(this.filePath))
140149
return {
141150
success: false,
151+
knownReason: true,
142152
reason: {
143153
user: `File path is an absolute path`,
144154
tech: `File path "${this.filePath}" is an absolute path`,
@@ -151,6 +161,7 @@ export class FileShareAccessorHandle<Metadata> extends GenericFileAccessorHandle
151161
if (!fullPath.startsWith(folderPath))
152162
return {
153163
success: false,
164+
knownReason: true,
154165
reason: {
155166
user: `File path is outside of folder path`,
156167
tech: `Full path "${fullPath}" does not start with "${folderPath}"`,
@@ -198,18 +209,21 @@ export class FileShareAccessorHandle<Metadata> extends GenericFileAccessorHandle
198209
if (err && (err as any).code === 'EBUSY') {
199210
return {
200211
success: false,
212+
knownReason: true,
201213
packageExists: true,
202214
reason: { user: `Not able to read file (file is busy)`, tech: `${stringifyError(err, true)}` },
203215
}
204216
} else if (err && (err as any).code === 'ENOENT') {
205217
return {
206218
success: false,
219+
knownReason: true,
207220
packageExists: false,
208221
reason: { user: `File does not exist`, tech: `${stringifyError(err, true)}` },
209222
}
210223
} else {
211224
return {
212225
success: false,
226+
knownReason: false,
213227
packageExists: false,
214228
reason: { user: `Not able to read file`, tech: `${stringifyError(err, true)}` },
215229
}
@@ -227,6 +241,7 @@ export class FileShareAccessorHandle<Metadata> extends GenericFileAccessorHandle
227241
// File is not readable
228242
return {
229243
success: false,
244+
knownReason: true,
230245
reason: {
231246
user: `File doesn't exist`,
232247
tech: `Not able to read file: ${stringifyError(err, true)}`,
@@ -244,6 +259,7 @@ export class FileShareAccessorHandle<Metadata> extends GenericFileAccessorHandle
244259
// File is not writeable
245260
return {
246261
success: false,
262+
knownReason: true,
247263
reason: {
248264
user: `Not able to write to container folder`,
249265
tech: `Not able to write to container folder: ${stringifyError(err, true)}`,
@@ -384,7 +400,7 @@ export class FileShareAccessorHandle<Metadata> extends GenericFileAccessorHandle
384400
}
385401

386402
if (!badReason) return { success: true }
387-
else return { success: false, reason: badReason }
403+
else return { success: false, knownReason: false, reason: badReason }
388404
}
389405
async setupPackageContainerMonitors(
390406
packageContainerExp: PackageContainerExpectation
@@ -653,6 +669,7 @@ export class FileShareAccessorHandle<Metadata> extends GenericFileAccessorHandle
653669
// File is not writeable
654670
return {
655671
success: false,
672+
knownReason: false,
656673
reason: {
657674
user: `Not able to read from container folder`,
658675
tech: `Not able to read from container folder: ${stringifyError(err, true)}`,

‎shared/packages/worker/src/worker/accessorHandlers/genericHandle.ts

+7
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
MonitorId,
99
promiseTimeout,
1010
INNER_ACTION_TIMEOUT,
11+
KnownReason,
1112
} from '@sofie-package-manager/api'
1213
import { BaseWorker } from '../worker'
1314
import { MonitorInProgress } from '../lib/monitorInProgress'
@@ -215,6 +216,12 @@ type AccessorHandlerResultSuccess = {
215216
type AccessorHandlerResultBad = {
216217
/** Whether the action was successful or not */
217218
success: false
219+
/**
220+
* This is set to true if the reason for being unsuccessful is well known.
221+
* If set to false, this means that there is a chance that the error originates from a Package Manager Worker,
222+
* so if this happens enough times, a worker might eventually be restarted.
223+
* */
224+
knownReason: KnownReason
218225
/** The reason why the action wasn't successful*/
219226
reason: Reason
220227
}

‎shared/packages/worker/src/worker/accessorHandlers/http.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ export class HTTPAccessorHandle<Metadata> extends GenericAccessorHandle<Metadata
6969
if (this.accessor.type !== Accessor.AccessType.HTTP) {
7070
return {
7171
success: false,
72+
knownReason: false,
7273
reason: {
7374
user: `There is an internal issue in Package Manager`,
7475
tech: `HTTP Accessor type is not HTTP ("${this.accessor.type}")!`,
@@ -82,6 +83,7 @@ export class HTTPAccessorHandle<Metadata> extends GenericAccessorHandle<Metadata
8283
if (!this.path)
8384
return {
8485
success: false,
86+
knownReason: true,
8587
reason: {
8688
user: `filePath not set`,
8789
tech: `filePath not set`,
@@ -107,6 +109,7 @@ export class HTTPAccessorHandle<Metadata> extends GenericAccessorHandle<Metadata
107109
if (this.isBadHTTPResponseCode(header.status)) {
108110
return {
109111
success: false,
112+
knownReason: true,
110113
reason: {
111114
user: `Got error code ${header.status} when trying to fetch package`,
112115
tech: `Error when requesting url "${this.fullUrl}": [${header.status}]: ${header.statusText}`,
@@ -213,7 +216,7 @@ export class HTTPAccessorHandle<Metadata> extends GenericAccessorHandle<Metadata
213216
}
214217

215218
if (!badReason) return { success: true }
216-
else return { success: false, reason: badReason }
219+
else return { success: false, knownReason: false, reason: badReason }
217220
}
218221
async setupPackageContainerMonitors(
219222
packageContainerExp: PackageContainerExpectation

‎shared/packages/worker/src/worker/accessorHandlers/httpProxy.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ export class HTTPProxyAccessorHandle<Metadata> extends GenericAccessorHandle<Met
7070
if (this.accessor.type !== Accessor.AccessType.HTTP_PROXY) {
7171
return {
7272
success: false,
73+
knownReason: false,
7374
reason: {
7475
user: `There is an internal issue in Package Manager`,
7576
tech: `HTTPProxy Accessor type is not HTTP_PROXY ("${this.accessor.type}")!`,
@@ -79,6 +80,7 @@ export class HTTPProxyAccessorHandle<Metadata> extends GenericAccessorHandle<Met
7980
if (!this.accessor.baseUrl)
8081
return {
8182
success: false,
83+
knownReason: true,
8284
reason: {
8385
user: `Accessor baseUrl not set`,
8486
tech: `Accessor baseUrl not set`,
@@ -88,6 +90,7 @@ export class HTTPProxyAccessorHandle<Metadata> extends GenericAccessorHandle<Met
8890
if (!this.filePath)
8991
return {
9092
success: false,
93+
knownReason: true,
9194
reason: {
9295
user: `filePath not set`,
9396
tech: `filePath not set`,
@@ -112,6 +115,7 @@ export class HTTPProxyAccessorHandle<Metadata> extends GenericAccessorHandle<Met
112115
if (this.isBadHTTPResponseCode(header.status)) {
113116
return {
114117
success: false,
118+
knownReason: true,
115119
reason: {
116120
user: `Got error code ${header.status} when trying to fetch package`,
117121
tech: `Error when requesting url "${this.fullUrl}": [${header.status}]: ${header.statusText}`,
@@ -244,7 +248,7 @@ export class HTTPProxyAccessorHandle<Metadata> extends GenericAccessorHandle<Met
244248
}
245249

246250
if (!badReason) return { success: true }
247-
else return { success: false, reason: badReason }
251+
else return { success: false, knownReason: false, reason: badReason }
248252
}
249253
async setupPackageContainerMonitors(
250254
packageContainerExp: PackageContainerExpectation

‎shared/packages/worker/src/worker/accessorHandlers/lib/lib.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ export function defaultCheckHandleRead(
77
if (!accessor.allowRead) {
88
return {
99
success: false,
10+
knownReason: true,
1011
reason: {
11-
user: `Not allowed to read`,
12+
user: `Not allowed to read (in configuration)`,
1213
tech: `Not allowed to read (check PackageContainer settings)`,
1314
},
1415
}
@@ -21,8 +22,9 @@ export function defaultCheckHandleWrite(
2122
if (!accessor.allowWrite) {
2223
return {
2324
success: false,
25+
knownReason: true,
2426
reason: {
25-
user: `Not allowed to write`,
27+
user: `Not allowed to write (in configuration)`,
2628
tech: `Not allowed to write (check PackageContainer settings)`,
2729
},
2830
}

0 commit comments

Comments
 (0)