Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/xrpl/HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr

## Unreleased

### Fixed
* Better error handling in the `Client` for edge case errors

## 4.6.0 (2026-02-12)

### Added
Expand All @@ -15,7 +18,8 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr

### Added
* Support for `Lending Protocol` (XLS-66d).
* Export signing and binary codec utilities.
* Export signing and binary co
* dec utilities.

### Fixed
* Update ripple-binary-codec to 2.5.1 to address serialization/deserialization issues in `Issue` serialized type for `MPTIssue`.
Expand Down
19 changes: 18 additions & 1 deletion packages/xrpl/src/client/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,8 @@ export class Connection extends EventEmitter {
*
* @param message - The message received from the server.
*/

// eslint-disable-next-line max-lines-per-function, complexity -- Message handler needs to work with a lot of data
private onMessage(message): void {
this.trace('receive', message)
let data: Record<string, unknown>
Expand All @@ -349,7 +351,7 @@ export class Connection extends EventEmitter {
}
if (data.type == null && data.error) {
// e.g. slowDown
this.emit('error', data.error, data.error_message, data)
this.emit('error', data.error, data.error_message ?? data.error, data)
return
}
if (data.type) {
Expand All @@ -367,6 +369,21 @@ export class Connection extends EventEmitter {
}
}
}
if (data.type === 'error' && data.value != null) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- needed
const parsedValue: Record<string, unknown> = JSON.parse(
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
data.value as string,
)
if (parsedValue.id != null) {
this.requestManager.reject(
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
parsedValue.id as string | number,
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
new Error(data.error as string),
)
}
}
Comment on lines +372 to +386
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Wrap JSON.parse in a try-catch to handle malformed data.value.

If data.value contains invalid JSON, JSON.parse will throw an uncaught exception. While rippled should always send valid JSON, defensive error handling would prevent the client from crashing on unexpected malformed responses.

🛡️ Proposed fix
     if (data.type === 'error' && data.value != null) {
-      // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- needed
-      const parsedValue: Record<string, unknown> = JSON.parse(
-        // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
-        data.value as string,
-      )
-      if (parsedValue.id != null) {
-        this.requestManager.reject(
-          // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
-          parsedValue.id as string | number,
-          // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
-          new Error(data.error as string),
-        )
+      try {
+        // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- needed
+        const parsedValue: Record<string, unknown> = JSON.parse(
+          // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
+          data.value as string,
+        )
+        if (parsedValue.id != null) {
+          this.requestManager.reject(
+            // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
+            parsedValue.id as string | number,
+            // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
+            new Error(data.error as string),
+          )
+        }
+      } catch (error) {
+        if (error instanceof Error) {
+          this.emit('error', 'badMessage', error.message, data)
+        }
       }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (data.type === 'error' && data.value != null) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- needed
const parsedValue: Record<string, unknown> = JSON.parse(
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
data.value as string,
)
if (parsedValue.id != null) {
this.requestManager.reject(
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
parsedValue.id as string | number,
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
new Error(data.error as string),
)
}
}
if (data.type === 'error' && data.value != null) {
try {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- needed
const parsedValue: Record<string, unknown> = JSON.parse(
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
data.value as string,
)
if (parsedValue.id != null) {
this.requestManager.reject(
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
parsedValue.id as string | number,
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- Should be true
new Error(data.error as string),
)
}
} catch (error) {
if (error instanceof Error) {
this.emit('error', 'badMessage', error.message, data)
}
}
}

}

/**
Expand Down
20 changes: 20 additions & 0 deletions packages/xrpl/test/connection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { assert } from 'chai'
import { HttpsProxyAgent } from 'https-proxy-agent'

import {
AccountInfoRequest,
Client,
ConnectionError,
DisconnectedError,
Expand Down Expand Up @@ -1028,4 +1029,23 @@ describe('Connection', function () {
},
TIMEOUT,
)

it('should handle jsonInvalid errors', async function () {
const response = {
error: 'jsonInvalid',
type: 'error',
value: '{"command":"submit","tx_json":"badjson"}',
}
// doesn't matter what the request is, since we're mocking the response
const request: AccountInfoRequest = {
command: 'account_info',
account: 'rQ3PTWGLCbPz8ZCicV5tCX3xuymojTng5r',
}
clientContext.mockRippled?.addResponse(request.command, response)

await clientContext.client.request(request).catch((error) => {
assert.strictEqual(error.name, 'Error')
assert.strictEqual(error.message, 'jsonInvalid')
})
}, 4000)
})
6 changes: 6 additions & 0 deletions packages/xrpl/test/createMockRippled.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ export function createResponse(
)}`,
)
}
if (response.type === 'error' && 'value' in response) {
const value = JSON.parse(response.value as string)
value.id = request.id
response.value = JSON.stringify(value)
return JSON.stringify(response)
}
return JSON.stringify({ ...response, id: request.id })
}

Expand Down
Loading