fix: make submitAndWait fail faster with tem errors
#2948
Conversation
WalkthroughThe PR updates several key components in the XRPL package. In the HISTORY file, an entry now documents that the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that transactions resulting in tem errors fail fast in submitAndWait, reducing unnecessary wait times.
- Introduces an early error check in submitAndWait for tem errors.
- Adds a new test case using a tem error fixture to verify the fast failure behavior.
- Updates HISTORY.md to document the change.
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/xrpl/test/fixtures/rippled/index.ts | Added tem error fixture to support the tem error test. |
| packages/xrpl/test/client/submitAndWait.test.ts | Added test case to assert that submitAndWait fails fast with tem errors. |
| packages/xrpl/src/client/index.ts | Introduced simulate() method and updated submitAndWait to throw on tem errors. |
| packages/xrpl/HISTORY.md | Updated changelog with a note on failing faster for tem errors. |
Files not reviewed (1)
- packages/xrpl/test/fixtures/rippled/submitTemError.json: Language not supported
Comments suppressed due to low confidence (2)
packages/xrpl/src/client/index.ts:847
- [nitpick] The function is named submitAndWait, so the comment might be clearer if it referenced submitAndWait rather than submit.
* This is similar to submit, which does all of the above, but also waits to see if the transaction has been validated.
packages/xrpl/src/client/index.ts:740
- The new simulate() method does not have any test coverage. Consider adding tests to ensure its behavior is correct.
public async simulate<Binary extends boolean = false>(
| @@ -720,6 +720,41 @@ class Client extends EventEmitter<EventTypes> { | |||
| return Promise.all(promises).then(() => tx) | |||
| } | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
This is just moved, no code change
| @@ -0,0 +1,44 @@ | |||
| import { XrplError } from '../../src' | |||
There was a problem hiding this comment.
This file (packages/xrpl/test/integration/submitAndWait.test.ts) is also relevant for adding this test case. Why did you create a new file?
There was a problem hiding this comment.
It's easier to test this sort of thing with a unit test where I can control the fixture as opposed to with an integration test.
High Level Overview of Change
Title says it all.
temerrors will never be validated on the XRPL, so no need to wait to see if they will. This will save a lot of waiting time on anytemerrors.Context of Change
Takes inspiration from XRPLF/xrpl-py#379 and XRPLF/xrpl-py#590 (which was for some reason not implemented in JS at the time)
Type of Change
Did you update HISTORY.md?
Test Plan
Added a test. CI passes.