Check if isCompleted is undefined #315
Conversation
| @@ -36,6 +36,7 @@ | |||
| }, | |||
| "devDependencies": { | |||
| "@multiversx/sdk-network-providers": "1.2.1", | |||
| "@multiversx/sdk-network-providers-next": "npm:@multiversx/sdk-network-providers@1.6.0-beta.0", | |||
There was a problem hiding this comment.
All right for now (to reference the beta version), since it's used for tests only (dev dependency). Once sdk-core is compatible with both the old and the new sdk-network-providers (as of this PR), we can make a proper release there, as well. And afterwards, come back here to adjust the reference.
For sdk-core, let's bump the minor version.
There was a problem hiding this comment.
Ok, got it. So after this PR is merged we make a proper release for sdk-network-providers, then come back here and reference the proper release. Will also make a new release for sdk-core increasing the minor version. 👍🏻
There was a problem hiding this comment.
First, release of this adjustment of sdk-core, then release sdk-network-providers. Afterwards, come back in sdk-core to adjust the version (this can be delayed anyway).
src/errors.ts
Outdated
|
|
||
| export class ErrIsCompletedFieldIsMissingOnTransaction extends Err { | ||
| public constructor() { | ||
| super("The transaction watcher requires the isCompleted property to be defined on the transaction object. Perhaps you've used the sdk-network-provider's getTransaction() and in that case you should also pass `withProcessStatus=true`.") |
There was a problem hiding this comment.
Perhaps add quotes for isCompleted and getTransaction(), as did for withProcessStatus.
| it("should send transactions", async function () { | ||
| this.timeout(30000); | ||
| it("should send transactions and wait for completion", async function () { | ||
| this.timeout(70000); |
There was a problem hiding this comment.
Yup. Because the completion checks are now much more strict, multiversx/mx-chain-proxy-go#386, and the wait time is increased.
Let's add links towards the following PRs in the description:
| let provider = createLocalnetProvider(); | ||
| let newProvider = new ProxyNetworkProvider("http://localhost:7950", { timeout: 5000 }); | ||
| let watcher = new TransactionWatcher({ | ||
| getTransaction: async (hash: string) => { return await newProvider.getTransaction(hash, true) } |
There was a problem hiding this comment.
This snippet should be added in the PR description (and in the release notes).
There was a problem hiding this comment.
Added in the PR description. Will also add in the release nodes when it's time.
| @@ -155,6 +161,10 @@ export class TransactionWatcher { | |||
| } catch (error) { | |||
| Logger.debug("TransactionWatcher.awaitConditionally(): cannot (yet) fetch data."); | |||
|
|
|||
| if (error instanceof ErrIsCompletedFieldIsMissingOnTransaction) { | |||
There was a problem hiding this comment.
This is correct.
@CiprianDraghici can confirm, as well.
This is done because in the new version of
sdk-network-providerstheisCompletedfield has become optional. Nothing has changed for theApiNetworkProvider, but for theProxyNetworkProvidertheisCompletedfield will only be set ifgetTransaction()is called usingwithProcessStatus=true.As you may know, the
TransactionWatcherclass needs aTransactionFetcherobject when initialized. You can use bothApiNetworkProviderorProxyNetworkProviderbecause both respect the interface.In case you are using the
proxyprovider and you need theisCompletedfield, a plain object that uses the proxy provider can be passed, defining agetTransaction()method that explicitly callsproxyProvider.getTransaction()usingwithProcessStatus=true. This can be done as bellow:Now, the completion checks are more strict, so it takes more time to check if a transaction is completed. The changes have been done in the following PR's: