Fix issue #2858: Improve error messages in field validation functions

packages/xrpl/src/models/transactions/common.ts

@@ -210,20 +210,22 @@ export function isXChainBridge(input: unknown): input is XChainBridge {
   )
 }
 
-/* eslint-disable @typescript-eslint/restrict-template-expressions -- tx.TransactionType is checked before any calls */
-
+/* eslint-disable max-params -- Allowing 4 params for better error messages */
+/* eslint-disable @typescript-eslint/restrict-template-expressions -- Using template expressions with checked values */
 /**
  * Verify the form and type of a required type for a transaction at runtime.
  *
  * @param tx - The transaction input to check the form and type of.
  * @param paramName - The name of the transaction parameter.
  * @param checkValidity - The function to use to check the type.
- * @throws
+ * @param expectedType - Optional. The expected type for more specific error messages.
+ * @throws When the field is missing or invalid.
  */
 export function validateRequiredField(
   tx: Record<string, unknown>,
   paramName: string,
   checkValidity: (inp: unknown) => boolean,
+  expectedType?: string,
 ): void {
   if (tx[paramName] == null) {
     throw new ValidationError(
@@ -232,8 +234,11 @@ export function validateRequiredField(
   }
 
   if (!checkValidity(tx[paramName])) {
+    const actualType = tx[paramName] === null ? 'null' : typeof tx[paramName]
     throw new ValidationError(
-      `${tx.TransactionType}: invalid field ${paramName}`,
+      `${tx.TransactionType}: invalid field ${paramName}${
+        expectedType ? `: expected ${expectedType}, received ${actualType}` : ''
+      }`,
     )
   }
 }
@@ -244,21 +249,26 @@ export function validateRequiredField(
  * @param tx - The transaction input to check the form and type of.
  * @param paramName - The name of the transaction parameter.
  * @param checkValidity - The function to use to check the type.
- * @throws
+ * @param expectedType - Optional. The expected type for more specific error messages.
+ * @throws When the field is invalid.
  */
 export function validateOptionalField(
   tx: Record<string, unknown>,
   paramName: string,
   checkValidity: (inp: unknown) => boolean,
+  expectedType?: string,
 ): void {
   if (tx[paramName] !== undefined && !checkValidity(tx[paramName])) {
+    const actualType = tx[paramName] === null ? 'null' : typeof tx[paramName]
     throw new ValidationError(
-      `${tx.TransactionType}: invalid field ${paramName}`,
+      `${tx.TransactionType}: invalid field ${paramName}${
+        expectedType ? `: expected ${expectedType}, received ${actualType}` : ''
+      }`,
     )
   }
 }
-
-/* eslint-enable @typescript-eslint/restrict-template-expressions -- checked before */
+/* eslint-enable max-params */
+/* eslint-enable @typescript-eslint/restrict-template-expressions */
 
 // eslint-disable-next-line @typescript-eslint/no-empty-interface -- no global flags right now, so this is fine
 export interface GlobalFlags {}
@@ -360,15 +370,15 @@ export function validateBaseTransaction(common: Record<string, unknown>): void {
     throw new ValidationError('BaseTransaction: Unknown TransactionType')
   }
 
-  validateRequiredField(common, 'Account', isString)
+  validateRequiredField(common, 'Account', isString, 'string')
 
-  validateOptionalField(common, 'Fee', isString)
+  validateOptionalField(common, 'Fee', isString, 'string')
 
-  validateOptionalField(common, 'Sequence', isNumber)
+  validateOptionalField(common, 'Sequence', isNumber, 'number')
 
-  validateOptionalField(common, 'AccountTxnID', isString)
+  validateOptionalField(common, 'AccountTxnID', isString, 'string')
 
-  validateOptionalField(common, 'LastLedgerSequence', isNumber)
+  validateOptionalField(common, 'LastLedgerSequence', isNumber, 'number')
 
   // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Only used by JS
   const memos = common.Memos as Array<{ Memo?: unknown }> | undefined
@@ -386,15 +396,15 @@ export function validateBaseTransaction(common: Record<string, unknown>): void {
     throw new ValidationError('BaseTransaction: invalid Signers')
   }
 
-  validateOptionalField(common, 'SourceTag', isNumber)
+  validateOptionalField(common, 'SourceTag', isNumber, 'number')
 
-  validateOptionalField(common, 'SigningPubKey', isString)
+  validateOptionalField(common, 'SigningPubKey', isString, 'string')
 
-  validateOptionalField(common, 'TicketSequence', isNumber)
+  validateOptionalField(common, 'TicketSequence', isNumber, 'number')
 
-  validateOptionalField(common, 'TxnSignature', isString)
+  validateOptionalField(common, 'TxnSignature', isString, 'string')
 
-  validateOptionalField(common, 'NetworkID', isNumber)
+  validateOptionalField(common, 'NetworkID', isNumber, 'number')
 }
 
 /**

packages/xrpl/test/.eslintrc.js

@@ -0,0 +1,5 @@
+module.exports = {
+  rules: {
+    'jsdoc/require-jsdoc': 'off',
+  },
+};

packages/xrpl/test/models/transactions/common.test.ts

@@ -0,0 +1,92 @@
+import { ValidationError } from '../../../src/errors'
+import {
+  validateRequiredField,
+  validateOptionalField,
+} from '../../../src/models/transactions/common'
+
+describe('validateRequiredField', () => {
+  const txMock = {
+    TransactionType: 'Payment',
+    amount: 42,
+    account: 'rXYZ',
+  }
+
+  it('throws an error with expected and actual types', () => {
+    expect(() =>
+      validateRequiredField(
+        txMock,
+        'amount',
+        (val) => typeof val === 'string',
+        'string',
+      ),
+    ).toThrow(
+      new ValidationError(
+        'Payment: invalid field amount: expected string, received number',
+      ),
+    )
+  })
+
+  it('does not throw if value is valid', () => {
+    expect(() =>
+      validateRequiredField(
+        txMock,
+        'account',
+        (val) => typeof val === 'string',
+        'string',
+      ),
+    ).not.toThrow()
+  })
+
+  it('throws without expectedType if not passed', () => {
+    expect(() =>
+      validateRequiredField(txMock, 'amount', (val) => typeof val === 'string'),
+    ).toThrow(new ValidationError('Payment: invalid field amount'))
+  })
+
+  it('throws when field is missing', () => {
+    expect(() =>
+      validateRequiredField(
+        txMock,
+        'nonExistentField',
+        (val) => typeof val === 'string',
+      ),
+    ).toThrow(new ValidationError('Payment: missing field nonExistentField'))
+  })
+})
+
+describe('validateOptionalField', () => {
+  const txMock = {
+    TransactionType: 'Payment',
+    memo: 123,
+  }
+
+  const txNoMemo = {
+    TransactionType: 'Payment',
+  }
+
+  it('skips validation if value is undefined', () => {
+    expect(() =>
+      validateOptionalField(
+        txNoMemo,
+        'memo',
+        (val) => typeof val === 'string',
+        'string',
+      ),
+    ).not.toThrow()
+  })
+
+  it('delegates to validation if value is defined', () => {
+    expect(() =>
+      validateOptionalField(
+        txMock,
+        'memo',
+        (val) => typeof val === 'string',
+        'string',
+      ),
+    ).toThrow(
+      new ValidationError(
+        'Payment: invalid field memo: expected string, received number',
+      ),
+    )
+  })
+})