Merge pull request #1621 from bbc/feat/SOFIE-295

feat: add informative REST results for TAKE failures (SOFIE-295)

Author: anteeek

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

@@ -117,6 +117,7 @@ interface APIRequestError {
 	status: number
 	message: string
 	details?: string[]
+	additionalInfo?: Record<string, unknown>
 }
 
 function sofieAPIRequest<API, Params, Body, Response>(
@@ -133,6 +134,7 @@ function sofieAPIRequest<API, Params, Body, Response>(
 	) => Promise<ClientAPI.ClientResponse<Response>>
 ) {
 	koaRouter[method](route, async (ctx, next) => {
+		let responseAdditionalInfo: Record<string, unknown> | undefined
 		try {
 			const context = new APIContext()
 			const serverAPI = serverAPIFactory.createServerAPI(context)
@@ -144,6 +146,7 @@ function sofieAPIRequest<API, Params, Body, Response>(
 				ctx.request.body as unknown as Body
 			)
 			if (ClientAPI.isClientResponseError(response)) {
+				responseAdditionalInfo = response.additionalInfo
 				throw UserError.fromSerialized(response.error)
 			}
 			ctx.body = JSON.stringify({ status: response.success, result: response.result })
@@ -176,7 +179,8 @@ function sofieAPIRequest<API, Params, Body, Response>(
 			ctx.type = 'application/json'
 			const bodyObj: APIRequestError = { status: errCode, message: errMsg }
 			const details = extractErrorDetails(e)
-			if (details) bodyObj['details'] = details
+			if (details) bodyObj.details = details
+			if (responseAdditionalInfo) bodyObj.additionalInfo = responseAdditionalInfo
 			ctx.body = JSON.stringify(bodyObj)
 			ctx.status = errCode
 		}

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

@@ -32,7 +32,7 @@ import {
 } from '../../../collections'
 import { DBRundownPlaylist } from '@sofie-automation/corelib/dist/dataModel/RundownPlaylist'
 import { ServerClientAPI } from '../../client'
-import { QueueNextSegmentResult, StudioJobs } from '@sofie-automation/corelib/dist/worker/studio'
+import { QueueNextSegmentResult, StudioJobs, TakeNextPartResult } from '@sofie-automation/corelib/dist/worker/studio'
 import { getCurrentTime } from '../../../lib/lib'
 import { TriggerReloadDataResponse } from '@sofie-automation/meteor-lib/dist/api/userActions'
 import { ServerRundownAPI } from '../../rundown'
@@ -559,7 +559,7 @@ class PlaylistsServerAPI implements PlaylistsRestAPI {
 		event: string,
 		rundownPlaylistId: RundownPlaylistId,
 		fromPartInstanceId: PartInstanceId | undefined
-	): Promise<ClientAPI.ClientResponse<void>> {
+	): Promise<ClientAPI.ClientResponse<TakeNextPartResult>> {
 		triggerWriteAccess()
 		const playlist = await this.findPlaylist(rundownPlaylistId)
 
@@ -923,7 +923,7 @@ export function registerRoutes(registerRoute: APIRegisterHook<PlaylistsRestAPI>)
 		}
 	)
 
-	registerRoute<{ playlistId: string }, { fromPartInstanceId?: string }, void>(
+	registerRoute<{ playlistId: string }, { fromPartInstanceId?: string }, TakeNextPartResult>(
 		'post',
 		'/playlists/:playlistId/take',
 		new Map([

meteor/server/lib/rest/v1/playlists.ts

@@ -11,7 +11,7 @@ import {
 	RundownPlaylistId,
 	SegmentId,
 } from '@sofie-automation/corelib/dist/dataModel/Ids'
-import { QueueNextSegmentResult } from '@sofie-automation/corelib/dist/worker/studio'
+import { QueueNextSegmentResult, TakeNextPartResult } from '@sofie-automation/corelib/dist/worker/studio'
 import { Meteor } from 'meteor/meteor'
 
 /* *************************************************************************
@@ -238,7 +238,7 @@ export interface PlaylistsRestAPI {
 		event: string,
 		rundownPlaylistId: RundownPlaylistId,
 		fromPartInstanceId: PartInstanceId | undefined
-	): Promise<ClientAPI.ClientResponse<void>>
+	): Promise<ClientAPI.ClientResponse<TakeNextPartResult>>
 	/**
 	 * Clears the specified SourceLayers.
 	 *

packages/corelib/src/worker/studio.ts

@@ -381,6 +381,10 @@ export interface CleanupOrphanedExpectedPackageReferencesProps {
 	rundownId: RundownId
 }
 
+export interface TakeNextPartResult {
+	nextTakeTime: number
+}
+
 /**
  * Set of valid functions, of form:
  * `id: (data) => return`
@@ -405,7 +409,7 @@ export type StudioJobFunc = {
 	[StudioJobs.QueueNextSegment]: (data: QueueNextSegmentProps) => QueueNextSegmentResult
 	[StudioJobs.ExecuteAction]: (data: ExecuteActionProps) => ExecuteActionResult
 	[StudioJobs.ExecuteBucketAdLibOrAction]: (data: ExecuteBucketAdLibOrActionProps) => ExecuteActionResult
-	[StudioJobs.TakeNextPart]: (data: TakeNextPartProps) => void
+	[StudioJobs.TakeNextPart]: (data: TakeNextPartProps) => TakeNextPartResult
 	[StudioJobs.DisableNextPiece]: (data: DisableNextPieceProps) => void
 	[StudioJobs.RemovePlaylist]: (data: RemovePlaylistProps) => void
 	[StudioJobs.RegeneratePlaylist]: (data: RegeneratePlaylistProps) => void

packages/job-worker/src/playout/take.ts

@@ -40,10 +40,12 @@ import { PlayoutRundownModel } from './model/PlayoutRundownModel.js'
 import { convertNoteToNotification } from '../notifications/util.js'
 import { PersistentPlayoutStateStore } from '../blueprints/context/services/PersistantStateStore.js'
 
+import { TakeNextPartResult } from '@sofie-automation/corelib/dist/worker/studio'
+
 /**
  * Take the currently Next:ed Part (start playing it)
  */
-export async function handleTakeNextPart(context: JobContext, data: TakeNextPartProps): Promise<void> {
+export async function handleTakeNextPart(context: JobContext, data: TakeNextPartProps): Promise<TakeNextPartResult> {
 	const now = getCurrentTime()
 
 	return runJobWithPlayoutModel(
@@ -77,17 +79,29 @@ export async function handleTakeNextPart(context: JobContext, data: TakeNextPart
 				}
 			}
 			if (lastTakeTime && now - lastTakeTime < context.studio.settings.minimumTakeSpan) {
+				const nextTakeTime = lastTakeTime + context.studio.settings.minimumTakeSpan
 				logger.debug(
 					`Time since last take is shorter than ${context.studio.settings.minimumTakeSpan} for ${
 						playlist.currentPartInfo?.partInstanceId
 					}: ${now - lastTakeTime}`
 				)
-				throw UserError.create(UserErrorMessage.TakeRateLimit, {
-					duration: context.studio.settings.minimumTakeSpan,
-				})
+				throw UserError.create(
+					UserErrorMessage.TakeRateLimit,
+					{
+						duration: context.studio.settings.minimumTakeSpan,
+						nextAllowedTakeTime: nextTakeTime,
+					},
+					429
+				)
 			}
 
-			return performTakeToNextedPart(context, playoutModel, now, undefined)
+			const nextTakeTime = now + context.studio.settings.minimumTakeSpan
+
+			await performTakeToNextedPart(context, playoutModel, now, undefined)
+
+			return {
+				nextTakeTime,
+			}
 		}
 	)
 }
@@ -159,7 +173,14 @@ export async function performTakeToNextedPart(
 			logger.debug(
 				`Take is blocked until ${currentPartInstance.partInstance.blockTakeUntil}. Which is in: ${remainingTime}`
 			)
-			throw UserError.create(UserErrorMessage.TakeBlockedDuration, { duration: remainingTime })
+			throw UserError.create(
+				UserErrorMessage.TakeBlockedDuration,
+				{
+					duration: remainingTime,
+					nextAllowedTakeTime: currentPartInstance.partInstance.blockTakeUntil,
+				},
+				425
+			)
 		}
 
 		// If there was a transition from the previous Part, then ensure that has finished before another take is permitted
@@ -171,11 +192,17 @@ export async function performTakeToNextedPart(
 			start &&
 			now < start + currentPartInstance.partInstance.part.inTransition.blockTakeDuration
 		) {
-			throw UserError.create(UserErrorMessage.TakeDuringTransition)
+			throw UserError.create(
+				UserErrorMessage.TakeDuringTransition,
+				{
+					nextAllowedTakeTime: start + currentPartInstance.partInstance.part.inTransition.blockTakeDuration,
+				},
+				425
+			)
 		}
 
 		if (currentPartInstance.isTooCloseToAutonext(true)) {
-			throw UserError.create(UserErrorMessage.TakeCloseToAutonext)
+			throw UserError.create(UserErrorMessage.TakeCloseToAutonext, undefined, 425)
 		}
 	}
 

packages/meteor-lib/src/api/__tests__/client.test.ts

@@ -50,6 +50,24 @@ describe('ClientAPI', () => {
 			})
 		}
 	})
+	it('Extracts additionalInfo from error args', () => {
+		const error = ClientAPI.responseError(
+			UserError.create(
+				UserErrorMessage.TakeRateLimit,
+				{
+					duration: 1000,
+					nextAllowedTakeTime: 1234567890,
+				},
+				429
+			)
+		)
+		expect(error.additionalInfo).toEqual({ duration: 1000, nextAllowedTakeTime: 1234567890 })
+		expect(error.errorCode).toBe(429)
+	})
+	it('Does not include additionalInfo when no args', () => {
+		const error = ClientAPI.responseError(UserError.create(UserErrorMessage.InactiveRundown))
+		expect(error.additionalInfo).toBeUndefined()
+	})
 	describe('isClientResponseSuccess', () => {
 		it('Correctly recognizes a responseSuccess object', () => {
 			const response = ClientAPI.responseSuccess(undefined)

packages/meteor-lib/src/api/client.ts

@@ -50,6 +50,8 @@ export namespace ClientAPI {
 		errorCode: number
 		/** On error, provide a human-readable error message */
 		error: SerializedUserError
+		/** Optional additional information about the error, forwarded from UserError args */
+		additionalInfo?: Record<string, unknown>
 	}
 
 	/**
@@ -59,7 +61,12 @@ export namespace ClientAPI {
 	 * @returns A `ClientResponseError` object containing the error and the resolved error code.
 	 */
 	export function responseError(userError: UserError): ClientResponseError {
-		return { error: UserError.serialize(userError), errorCode: userError.errorCode }
+		const args = userError.userMessage.args
+		return {
+			error: UserError.serialize(userError),
+			errorCode: userError.errorCode,
+			...(args !== undefined && Object.keys(args).length > 0 && { additionalInfo: args }),
+		}
 	}
 	export interface ClientResponseSuccess<Result> {
 		/** On success, return success code (by default, use 200) */

packages/meteor-lib/src/api/userActions.ts

@@ -6,7 +6,11 @@ import { BucketAdLib } from '@sofie-automation/corelib/dist/dataModel/BucketAdLi
 import { AdLibActionCommon } from '@sofie-automation/corelib/dist/dataModel/AdlibAction'
 import { BucketAdLibAction } from '@sofie-automation/corelib/dist/dataModel/BucketAdLibAction'
 import { Time } from '@sofie-automation/blueprints-integration'
-import { ExecuteActionResult, QueueNextSegmentResult } from '@sofie-automation/corelib/dist/worker/studio'
+import {
+	ExecuteActionResult,
+	QueueNextSegmentResult,
+	TakeNextPartResult,
+} from '@sofie-automation/corelib/dist/worker/studio'
 import {
 	AdLibActionId,
 	BucketAdLibActionId,
@@ -34,7 +38,7 @@ export interface NewUserActionAPI {
 		eventTime: Time,
 		rundownPlaylistId: RundownPlaylistId,
 		fromPartInstanceId: PartInstanceId | null
-	): Promise<ClientAPI.ClientResponse<void>>
+	): Promise<ClientAPI.ClientResponse<TakeNextPartResult>>
 	setNext(
 		userEvent: string,
 		eventTime: Time,

packages/openapi/api/definitions/playlists.yaml

@@ -589,7 +589,22 @@ resources:
                   description: May be specified to ensure that multiple take requests from the same Part do not result in multiple takes.
       responses:
         200:
-          $ref: '#/components/responses/putSuccess'
+          description: Take was successful - returns the next allowed take time.
+          content:
+            application/json:
+              schema:
+                type: object
+                properties:
+                  status:
+                    type: number
+                    example: 200
+                  result:
+                    type: object
+                    properties:
+                      nextTakeTime:
+                        type: number
+                        description: Unix timestamp (ms) of when the next take will be allowed.
+                        example: 1707024000000
         404:
           $ref: '#/components/responses/playlistNotFound'
         412:
@@ -605,6 +620,40 @@ resources:
                   message:
                     type: string
                     example: No Next point found, please set a part as Next before doing a TAKE.
+        425:
+          description: Take is blocked due to a transition or adlib action.
+          content:
+            application/json:
+              schema:
+                type: object
+                properties:
+                  status:
+                    type: number
+                    example: 425
+                  message:
+                    type: string
+                    example: Cannot take during a transition
+                  additionalInfo:
+                    type: object
+                    description: Additional error details, e.g. includes nextAllowedTakeTime (Unix timestamp ms) for blocked takes.
+                    additionalProperties: true
+        429:
+          description: Take rate limit exceeded - takes are happening too quickly.
+          content:
+            application/json:
+              schema:
+                type: object
+                properties:
+                  status:
+                    type: number
+                    example: 429
+                  message:
+                    type: string
+                    example: Ignoring TAKES that are too quick after eachother (1000 ms)
+                  additionalInfo:
+                    type: object
+                    description: Additional error details, e.g. includes nextAllowedTakeTime (Unix timestamp ms) for blocked takes.
+                    additionalProperties: true
         500:
           $ref: '#/components/responses/internalServerError'