From 694fc4c7ccfb35ce021164cd8590e76453f7dd86 Mon Sep 17 00:00:00 2001
From: Igor <igor.ushkarev@digital.justice.gov.uk>
Date: Fri, 6 Oct 2023 15:47:26 +0100
Subject: [PATCH] Ensure api error responses always contain messages as well as
 HTTP status codes

---
 clients/node/src/errorTypes.ts                | 16 +++++++++++-----
 ...HmppsNonAssociationsApiExceptionHandler.kt | 19 ++++++++++---------
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/clients/node/src/errorTypes.ts b/clients/node/src/errorTypes.ts
index a20bfe4c..752468e2 100644
--- a/clients/node/src/errorTypes.ts
+++ b/clients/node/src/errorTypes.ts
@@ -5,11 +5,11 @@
 export interface ErrorResponse {
   status: number
 
-  errorCode?: ErrorCode
+  userMessage: string
 
-  userMessage?: string
+  developerMessage: string
 
-  developerMessage?: string
+  errorCode?: ErrorCode
 
   moreInfo?: string
 }
@@ -18,8 +18,14 @@ export interface ErrorResponse {
  * Used to determine if an api error was of type `ErrorResponse` to use relevant properties
  */
 export function isErrorResponse(obj: unknown): obj is ErrorResponse {
-  // TODO: would be nice to make userMessage & developerMessage non-nullable in the api
-  return Boolean(obj && typeof obj === 'object' && 'status' in obj && typeof obj.status === 'number')
+  return Boolean(
+    obj &&
+      typeof obj === 'object' &&
+      'status' in obj &&
+      typeof obj.status === 'number' &&
+      'userMessage' in obj &&
+      'developerMessage' in obj,
+  )
 }
 
 /**
diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/config/HmppsNonAssociationsApiExceptionHandler.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/config/HmppsNonAssociationsApiExceptionHandler.kt
index f8fbafae..5293ae77 100644
--- a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/config/HmppsNonAssociationsApiExceptionHandler.kt
+++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/config/HmppsNonAssociationsApiExceptionHandler.kt
@@ -132,13 +132,14 @@ class HmppsNonAssociationsApiExceptionHandler {
   @ExceptionHandler(ResponseStatusException::class)
   fun handleResponseStatusException(e: ResponseStatusException): ResponseEntity<ErrorResponse?>? {
     log.debug("Response status exception caught: {}", e.message)
+    val reason = e.reason ?: "Unknown error"
     return ResponseEntity
       .status(e.statusCode)
       .body(
         ErrorResponse(
           status = e.statusCode.value(),
-          userMessage = e.reason,
-          developerMessage = e.reason,
+          userMessage = reason,
+          developerMessage = reason,
         ),
       )
   }
@@ -240,23 +241,23 @@ enum class ErrorCode(val errorCode: Int) {
 data class ErrorResponse(
   @Schema(description = "HTTP status code", example = "500", required = true)
   val status: Int,
+  @Schema(description = "User message for the error", example = "No non-association found for ID `324234`", required = true)
+  val userMessage: String,
+  @Schema(description = "More detailed error message", example = "[Details, sometimes a stack trace]", required = true)
+  val developerMessage: String,
   @Schema(description = "When present, uniquely identifies the type of error making it easier for clients to discriminate without relying on error description or HTTP status code; see `uk.gov.justice.digital.hmpps.hmppsnonassociationsapi.config.ErrorCode` enumeration in hmpps-non-associations-api", example = "101", required = false)
   val errorCode: Int? = null,
-  @Schema(description = "User message for the error", example = "No non-association found for ID `324234`", required = false)
-  val userMessage: String? = null,
-  @Schema(description = "More detailed error message", example = "[Details, sometimes a stack trace]", required = false)
-  val developerMessage: String? = null,
   @Schema(description = "More information about the error", example = "[Rarely used, error-specific]", required = false)
   val moreInfo: String? = null,
 ) {
   constructor(
     status: HttpStatus,
-    errorCode: ErrorCode? = null,
-    userMessage: String? = null,
+    userMessage: String,
     developerMessage: String? = null,
+    errorCode: ErrorCode? = null,
     moreInfo: String? = null,
   ) :
-    this(status.value(), errorCode?.errorCode, userMessage, developerMessage, moreInfo)
+    this(status.value(), userMessage, developerMessage ?: userMessage, errorCode?.errorCode, moreInfo)
 }
 
 class NonAssociationAlreadyClosedException(id: Long) : Exception("Non-association [ID=$id] already closed")