Skip to content

Commit cd0b1a7

Browse files
jstarpljstarpl
authored
Merge pull request #1485 from SuperFlyTV/fix/http-api-status-codes-responses
2 parents e97730b + 502c7d3 commit cd0b1a7

File tree

27 files changed

+271
-135
lines changed

27 files changed

+271
-135
lines changed

‎meteor/__mocks__/_extendJest.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ expect.extend({
4242
received = received.error
4343
}
4444

45-
if (UserError.isUserError(received)) {
45+
if (received instanceof UserError) {
4646
const expected = UserError.create(msg, args)
4747
const received2 = clone(received)
4848

@@ -68,10 +68,11 @@ expect.extend({
6868
received = received.error
6969
}
7070

71-
if (UserError.isUserError(received)) {
72-
const pass = !!received.rawError.toString().match(regexp)
71+
if (UserError.isSerializedUserErrorObject(received)) {
72+
received = UserError.fromUnknown(received)
73+
const pass = !!received.toString().match(regexp)
7374
return {
74-
message: () => `expected ${received.rawError} to match ${regexp}`,
75+
message: () => `expected ${stringifyError(received)} to match ${regexp}`,
7576
pass: pass,
7677
}
7778
} else {

‎meteor/server/api/client.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,10 @@ import { assertConnectionHasOneOfPermissions } from '../security/auth'
3434

3535
function rewrapError(methodName: string, e: any): ClientAPI.ClientResponseError {
3636
const userError = UserError.fromUnknown(e)
37-
3837
logger.info(`UserAction "${methodName}" failed: ${userError.toErrorString()}`)
3938

4039
// Forward the error to the caller
41-
return ClientAPI.responseError(userError, userError.errorCode)
40+
return ClientAPI.responseError(userError)
4241
}
4342

4443
export namespace ServerClientAPI {
@@ -199,7 +198,6 @@ export namespace ServerClientAPI {
199198
// Just run and return right away:
200199
try {
201200
const result = await fcn({})
202-
203201
return ClientAPI.responseSuccess(result)
204202
} catch (e) {
205203
return rewrapError(methodName, e)

‎meteor/server/api/rest/v1/blueprints.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,12 @@ class BlueprintsServerAPI implements BlueprintsRestAPI {
3232
const blueprint = await Blueprints.findOneAsync(blueprintId)
3333
if (!blueprint) {
3434
return ClientAPI.responseError(
35-
UserError.from(new Error(`Blueprint ${blueprintId} not found`), UserErrorMessage.BlueprintNotFound),
36-
404
35+
UserError.from(
36+
new Error(`Blueprint ${blueprintId} not found`),
37+
UserErrorMessage.BlueprintNotFound,
38+
undefined,
39+
404
40+
)
3741
)
3842
}
3943

‎meteor/server/api/rest/v1/buckets.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,12 @@ export class BucketsServerAPI implements BucketsRestAPI {
3737
const bucket = await Buckets.findOneAsync(bucketId, { projection: { _id: 1, name: 1, studioId: 1 } })
3838
if (!bucket) {
3939
return ClientAPI.responseError(
40-
UserError.from(new Error(`Bucket ${bucketId} not found`), UserErrorMessage.BucketNotFound),
41-
404
40+
UserError.from(
41+
new Error(`Bucket ${bucketId} not found`),
42+
UserErrorMessage.BucketNotFound,
43+
undefined,
44+
404
45+
)
4246
)
4347
}
4448
return ClientAPI.responseSuccess(APIBucketFrom(bucket))

‎meteor/server/api/rest/v1/devices.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,10 @@ class DevicesServerAPI implements DevicesRestAPI {
3939
return ClientAPI.responseError(
4040
UserError.from(
4141
new Error(`Device ${deviceId} does not exist`),
42-
UserErrorMessage.PeripheralDeviceNotFound
43-
),
44-
404
42+
UserErrorMessage.PeripheralDeviceNotFound,
43+
undefined,
44+
404
45+
)
4546
)
4647
return ClientAPI.responseSuccess(APIPeripheralDeviceFrom(device))
4748
}
@@ -57,9 +58,10 @@ class DevicesServerAPI implements DevicesRestAPI {
5758
return ClientAPI.responseError(
5859
UserError.from(
5960
new Error(`Device ${deviceId} does not exist`),
60-
UserErrorMessage.PeripheralDeviceNotFound
61-
),
62-
404
61+
UserErrorMessage.PeripheralDeviceNotFound,
62+
undefined,
63+
404
64+
)
6365
)
6466

6567
switch (action.type) {

‎meteor/server/api/rest/v1/index.ts‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ koaRouter.use(bodyParser())
5050

5151
function extractErrorCode(e: unknown): number {
5252
if (ClientAPI.isClientResponseError(e)) {
53-
return e.errorCode
54-
} else if (UserError.isUserError(e)) {
53+
return e.error.errorCode
54+
} else if (UserError.isSerializedUserErrorObject(e) || e instanceof UserError) {
5555
return e.errorCode
5656
} else if ((e as Meteor.Error).error && typeof (e as Meteor.Error).error === 'number') {
5757
return (e as Meteor.Error).error as number
@@ -60,10 +60,18 @@ function extractErrorCode(e: unknown): number {
6060
}
6161
}
6262

63-
function extractErrorMessage(e: unknown): string {
63+
function validateUserError(e: unknown): UserError | undefined {
64+
if (e instanceof UserError) {
65+
return e
66+
} else if (UserError.isSerializedUserErrorObject(e)) {
67+
return UserError.fromUnknown(e)
68+
}
69+
}
70+
71+
function extractErrorUserMessage(e: unknown): string {
6472
if (ClientAPI.isClientResponseError(e)) {
6573
return translateMessage(e.error.userMessage, interpollateTranslation)
66-
} else if (UserError.isUserError(e)) {
74+
} else if (UserError.isSerializedUserErrorObject(e) || e instanceof UserError) {
6775
return translateMessage(e.userMessage, interpollateTranslation)
6876
} else if ((e as Meteor.Error).reason && typeof (e as Meteor.Error).reason === 'string') {
6977
return (e as Meteor.Error).reason as string
@@ -119,7 +127,7 @@ interface APIRequestError {
119127
function sofieAPIRequest<API, Params, Body, Response>(
120128
method: 'get' | 'post' | 'put' | 'delete',
121129
route: string,
122-
errMsgs: Map<number, UserErrorMessage[]>,
130+
errMsgFallbacks: Map<number, UserErrorMessage[]>,
123131
serverAPIFactory: APIFactory<API>,
124132
handler: (
125133
serverAPI: API,
@@ -140,27 +148,36 @@ function sofieAPIRequest<API, Params, Body, Response>(
140148
ctx.params as unknown as Params,
141149
ctx.request.body as unknown as Body
142150
)
143-
if (ClientAPI.isClientResponseError(response)) throw response.error
151+
if (ClientAPI.isClientResponseError(response)) {
152+
throw UserError.fromSerialized(response.error)
153+
}
144154
ctx.body = JSON.stringify({ status: response.success, result: response.result })
145155
ctx.status = response.success
146156
} catch (e) {
157+
const userError = validateUserError(e)
147158
const errCode = extractErrorCode(e)
148-
let errMsg = extractErrorMessage(e)
149-
const msgs = errMsgs.get(errCode)
150-
if (msgs) {
159+
let errMsg = extractErrorUserMessage(e)
160+
// Get the fallback messages of the endpoint
161+
const fallbackMsgs = errMsgFallbacks.get(errCode)
162+
163+
if (fallbackMsgs && (userError?.message === errMsg || userError?.message === '')) {
164+
// If no detailed error message is provided then return the fallback error messages.
151165
const msgConcat = {
152-
key: msgs
166+
key: fallbackMsgs
153167
.map((msg) => UserError.create(msg, undefined, errCode).userMessage.key)
154-
.reduce((acc, msg) => acc + (acc.length ? ' or ' : '') + msg, ''),
168+
.reduce((acc, msg) => acc + (acc.length ? ' or ' : '') + msg, errMsg),
155169
}
156170
errMsg = translateMessage(msgConcat, interpollateTranslation)
157-
} else {
158-
logger.error(
159-
`${method.toUpperCase()} for route ${route} returned unexpected error code ${errCode} - ${errMsg}`
160-
)
171+
} else if (userError?.message) {
172+
// If we have a detailed arbitrary error message then return that together with the standard error message.
173+
errMsg = `${errMsg}${userError.message !== errMsg && userError.message !== '' ? ` - ${userError?.message}` : ''}`
161174
}
162175

163-
logger.error(`${method.toUpperCase()} failed for route ${route}: ${errCode} - ${errMsg}`)
176+
// Log unknown error codes
177+
logger.error(
178+
`${method.toUpperCase()} failed for route ${route}:${!fallbackMsgs ? ' returned unexpected error code' : ''} ${errCode} - ${errMsg}`
179+
)
180+
164181
ctx.type = 'application/json'
165182
const bodyObj: APIRequestError = { status: errCode, message: errMsg }
166183
const details = extractErrorDetails(e)

‎meteor/server/api/rest/v1/playlists.ts‎

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -133,14 +133,19 @@ class PlaylistsServerAPI implements PlaylistsRestAPI {
133133
return ClientAPI.responseError(
134134
UserError.from(
135135
new Error(`Rundown playlist does not exist`),
136-
UserErrorMessage.RundownPlaylistNotFound
137-
),
138-
404
136+
UserErrorMessage.RundownPlaylistNotFound,
137+
undefined,
138+
404
139+
)
139140
)
140141
if (rundownPlaylist.currentPartInfo === null)
141142
return ClientAPI.responseError(
142-
UserError.from(Error(`No active Part in ${rundownPlaylistId}`), UserErrorMessage.PartNotFound),
143-
412
143+
UserError.from(
144+
Error(`No active Part in ${rundownPlaylistId}`),
145+
UserErrorMessage.PartNotFound,
146+
undefined,
147+
412
148+
)
144149
)
145150

146151
const result = await ServerClientAPI.runUserActionInLogForPlaylistOnWorker(
@@ -172,25 +177,28 @@ class PlaylistsServerAPI implements PlaylistsRestAPI {
172177
return ClientAPI.responseError(
173178
UserError.from(
174179
new Error(`Rundown playlist does not exist`),
175-
UserErrorMessage.RundownPlaylistNotFound
176-
),
177-
404
180+
UserErrorMessage.RundownPlaylistNotFound,
181+
undefined,
182+
404
183+
)
178184
)
179185
if (!rundownPlaylist.activationId)
180186
return ClientAPI.responseError(
181187
UserError.from(
182188
new Error(`Rundown playlist ${rundownPlaylistId} is not currently active`),
183-
UserErrorMessage.InactiveRundown
184-
),
185-
412
189+
UserErrorMessage.InactiveRundown,
190+
undefined,
191+
412
192+
)
186193
)
187194
if (!rundownPlaylist.currentPartInfo)
188195
return ClientAPI.responseError(
189196
UserError.from(
190197
new Error(`Rundown playlist ${rundownPlaylistId} must be playing`),
191-
UserErrorMessage.NoCurrentPart
192-
),
193-
412
198+
UserErrorMessage.NoCurrentPart,
199+
undefined,
200+
412
201+
)
194202
)
195203

196204
return ServerClientAPI.runUserActionInLogForPlaylistOnWorker(
@@ -214,8 +222,7 @@ class PlaylistsServerAPI implements PlaylistsRestAPI {
214222
)
215223
} else {
216224
return ClientAPI.responseError(
217-
UserError.from(new Error(`No adLib with Id ${adLibId}`), UserErrorMessage.AdlibNotFound),
218-
412
225+
UserError.from(new Error(`No adLib with Id ${adLibId}`), UserErrorMessage.AdlibNotFound, undefined, 412)
219226
)
220227
}
221228
}
@@ -242,17 +249,22 @@ class PlaylistsServerAPI implements PlaylistsRestAPI {
242249
])
243250
if (!bucket) {
244251
return ClientAPI.responseError(
245-
UserError.from(new Error(`Bucket ${bucketId} not found`), UserErrorMessage.BucketNotFound),
246-
412
252+
UserError.from(
253+
new Error(`Bucket ${bucketId} not found`),
254+
UserErrorMessage.BucketNotFound,
255+
undefined,
256+
412
257+
)
247258
)
248259
}
249260
if (!bucketAdlib && !bucketAdlibAction) {
250261
return ClientAPI.responseError(
251262
UserError.from(
252263
new Error(`No adLib with Id ${externalId}, in bucket ${bucketId}`),
253-
UserErrorMessage.AdlibNotFound
254-
),
255-
412
264+
UserErrorMessage.AdlibNotFound,
265+
undefined,
266+
412
267+
)
256268
)
257269
}
258270

@@ -477,17 +489,19 @@ class PlaylistsServerAPI implements PlaylistsRestAPI {
477489
return ClientAPI.responseError(
478490
UserError.from(
479491
Error(`Rundown playlist ${rundownPlaylistId} does not exist`),
480-
UserErrorMessage.RundownPlaylistNotFound
481-
),
482-
412
492+
UserErrorMessage.RundownPlaylistNotFound,
493+
undefined,
494+
412
495+
)
483496
)
484497
if (!rundownPlaylist.currentPartInfo?.partInstanceId || !rundownPlaylist.activationId)
485498
return ClientAPI.responseError(
486499
UserError.from(
487500
new Error(`Rundown playlist ${rundownPlaylistId} is not currently active`),
488-
UserErrorMessage.InactiveRundown
489-
),
490-
412
501+
UserErrorMessage.InactiveRundown,
502+
undefined,
503+
412
504+
)
491505
)
492506

493507
return ServerClientAPI.runUserActionInLogForPlaylistOnWorker(

‎meteor/server/api/rest/v1/studios.ts‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -256,24 +256,28 @@ class StudiosServerAPI implements StudiosRestAPI {
256256
const studio = await Studios.findOneAsync(studioId)
257257
if (!studio)
258258
return ClientAPI.responseError(
259-
UserError.from(new Error(`Studio does not exist`), UserErrorMessage.StudioNotFound),
260-
404
259+
UserError.from(new Error(`Studio does not exist`), UserErrorMessage.StudioNotFound, undefined, 404)
261260
)
262261

263262
const device = await PeripheralDevices.findOneAsync(deviceId)
264263
if (!device)
265264
return ClientAPI.responseError(
266-
UserError.from(new Error(`Studio does not exist`), UserErrorMessage.PeripheralDeviceNotFound),
267-
404
265+
UserError.from(
266+
new Error(`Studio does not exist`),
267+
UserErrorMessage.PeripheralDeviceNotFound,
268+
undefined,
269+
404
270+
)
268271
)
269272

270273
if (device.studioAndConfigId !== undefined && device.studioAndConfigId.studioId !== studio._id) {
271274
return ClientAPI.responseError(
272275
UserError.from(
273276
new Error(`Device already attached to studio`),
274-
UserErrorMessage.DeviceAlreadyAttachedToStudio
275-
),
276-
412
277+
UserErrorMessage.DeviceAlreadyAttachedToStudio,
278+
undefined,
279+
412
280+
)
277281
)
278282
}
279283

@@ -318,8 +322,7 @@ class StudiosServerAPI implements StudiosRestAPI {
318322
const studio = await Studios.findOneAsync(studioId)
319323
if (!studio)
320324
return ClientAPI.responseError(
321-
UserError.from(new Error(`Studio does not exist`), UserErrorMessage.StudioNotFound),
322-
404
325+
UserError.from(new Error(`Studio does not exist`), UserErrorMessage.StudioNotFound, undefined, 404)
323326
)
324327
await PeripheralDevices.updateAsync(
325328
{

‎meteor/server/worker/worker.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { MongoQuery } from '@sofie-automation/corelib/dist/mongo'
2222
import { UserActionsLog } from '../collections'
2323
import { MetricsCounter } from '@sofie-automation/corelib/dist/prometheus'
2424
import { isInTestWrite } from '../security/securityVerify'
25+
import { UserError } from '@sofie-automation/corelib/dist/error'
2526

2627
const FREEZE_LIMIT = 1000 // how long to wait for a response to a Ping
2728
const RESTART_TIMEOUT = 30000 // how long to wait for a restart to complete before throwing an error
@@ -107,7 +108,8 @@ async function jobFinished(
107108
}
108109

109110
if (job.completionHandler) {
110-
job.completionHandler(startedTime, finishedTime, err, result)
111+
const userError = err ? UserError.tryFromJSON(err) || new Error(err) : undefined
112+
job.completionHandler(startedTime, finishedTime, userError, result)
111113
}
112114
}
113115
}

‎packages/corelib/src/__tests__/error.spec.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ describe('UserError', () => {
1111
expect.stringContaining(
1212
'UserError: ' +
1313
JSON.stringify({
14-
rawError: 'Error: raw, mock stack',
14+
rawError: {
15+
name: 'UserError',
16+
message: 'raw',
17+
stack: 'mock stack',
18+
},
1519
userMessage: {
1620
key: 'The selected part does not exist',
1721
args: {

0 commit comments

Comments
 (0)