Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cli): add retry to getLogs when getting resource ID's #2709

Merged
merged 6 commits into from
Apr 23, 2024
Merged

fix(cli): add retry to getLogs when getting resource ID's #2709

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fb97fc9
fix(cli): add retry to getLogs when getting resource ID's
yonadaa Apr 23, 2024
a6ce365
refactor: use pRetry retry logic
yonadaa Apr 23, 2024
ade6164
feat: only retry if block out of range
yonadaa Apr 23, 2024
6da9640
chore: changeset
yonadaa Apr 23, 2024
a26f286
chore: changeset
yonadaa Apr 23, 2024
9e5a05a
refactor: remove quotes
yonadaa Apr 23, 2024
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
30 changes: 21 additions & 9 deletions packages/cli/src/deploy/getResourceIds.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { getLogs } from "viem/actions";
import { storeSpliceStaticDataEvent } from "@latticexyz/store";
import { WorldDeploy, storeTables } from "./common";
import { debug } from "./debug";
import pRetry from "p-retry";
import { wait } from "@latticexyz/common/utils";

export async function getResourceIds({
client,
Expand All @@ -15,15 +17,25 @@ export async function getResourceIds({
// TODO: PR to viem's getLogs to accept topics array so we can filter on all store events and quickly recreate this table's current state

debug("looking up resource IDs for", worldDeploy.address);
const logs = await getLogs(client, {
strict: true,
address: worldDeploy.address,
fromBlock: worldDeploy.deployBlock,
toBlock: worldDeploy.stateBlock,
event: parseAbiItem(storeSpliceStaticDataEvent),
args: { tableId: storeTables.store_ResourceIds.tableId },
});

const logs = await pRetry(
() =>
getLogs(client, {
strict: true,
address: worldDeploy.address,
fromBlock: worldDeploy.deployBlock,
toBlock: worldDeploy.stateBlock,
event: parseAbiItem(storeSpliceStaticDataEvent),
args: { tableId: storeTables.store_ResourceIds.tableId },
}),
{
retries: 3,
onFailedAttempt: async (error) => {
Copy link
Member

Choose a reason for hiding this comment

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

we could check the error to see if it contains "block is out of range" and only retry those? but I am happy just retrying all here since its a relatively safe operation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I also like that it makes the retry logic more self-explanatory

const delay = error.attemptNumber * 500;
debug(`failed to get logs, retrying in ${delay}ms...`);
await wait(delay);
Copy link
Member

Choose a reason for hiding this comment

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

no need for this here, I believe pRetry has its own back off internally
(I suspect this was copied from other spots where we also don't need to do this)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, yes just copied other examples :D

},
},
);
const resourceIds = logs.map((log) => log.args.keyTuple[0]);
debug("found", resourceIds.length, "resource IDs for", worldDeploy.address);

Expand Down
Loading