Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
chore: clean up
  • Loading branch information
xavier-brochard committed Aug 4, 2025
commit 144a0838e6bf7660d521992d951980de5b674f93
2 changes: 1 addition & 1 deletion packages/snap/snap.manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"url": "https://github.com/MetaMask/snap-solana-wallet.git"
},
"source": {
"shasum": "2uvwA8qWgOkAl3nzhJNnBp3IYOYsTb5jWaANNqZnXJo=",
"shasum": "8ufRG4iBVewqxExSv+Ri41hNIHkWbdWamb0dy9J08Cw=",
"location": {
"npm": {
"filePath": "dist/bundle.js",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,11 @@ import {
updateInterface,
} from '../../../utils/interface';
import baseLogger, { createPrefixedLogger } from '../../../utils/logger';
import { ScheduleBackgroundEventMethod } from './ScheduleBackgroundEventMethod';

export const refreshSend: OnCronjobHandler = async () => {
const logger = createPrefixedLogger(baseLogger, '[refreshSend]');

logger.info(
`[${ScheduleBackgroundEventMethod.RefreshSend}] Background event triggered`,
);
logger.info(`Background event triggered`);

const [assets, mapInterfaceNameToId, preferences] = await Promise.all([
assetsService.getAll(),
Expand All @@ -35,9 +32,7 @@ export const refreshSend: OnCronjobHandler = async () => {

// Don't do anything if the send form interface is not open
if (!sendFormInterfaceId) {
logger.info(
`[${ScheduleBackgroundEventMethod.RefreshSend}] ❌ No send form interface found`,
);
logger.info(`No send form interface found`);
return;
}

Expand All @@ -62,16 +57,12 @@ export const refreshSend: OnCronjobHandler = async () => {

// We only want to refresh the token prices when the user is in the transaction confirmation stage
if (interfaceContext.stage !== 'transaction-confirmation') {
logger.info(
`[${ScheduleBackgroundEventMethod.RefreshSend}] ❌ Not in transaction confirmation stage`,
);
logger.info(`❌ Not in transaction confirmation stage`);
return;
}

if (!interfaceContext.assets) {
logger.info(
`[${ScheduleBackgroundEventMethod.RefreshSend}] ❌ No assets found`,
);
logger.info(`❌ No assets found`);
return;
}

Expand All @@ -90,7 +81,5 @@ export const refreshSend: OnCronjobHandler = async () => {
updatedInterfaceContext,
);

logger.info(
`[${ScheduleBackgroundEventMethod.RefreshSend}] ✅ Background event suceeded`,
);
logger.info(`✅ Background event suceeded`);
};
Copy link

Choose a reason for hiding this comment

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

Bug

The refreshSend background event has two main issues related to its execution flow and error handling:

  1. Missing Error Handling: Critical operations, such as fetching token prices, saving them to state, and updating the interface, lack try-catch blocks. This differs from the previous cronjob version, which handled these failures gracefully.
  2. Incorrect Scheduling Order: The next background event is scheduled early in the handler, before checks for the send interface being open and before the token price fetching/state saving operations.

As a result:

  • If the send interface is closed, the event still schedules its next run before exiting, leading to an infinite loop of unnecessary background events.
  • If token price fetching or state saving fails, the current event throws an unhandled error, but the next event has already been scheduled, resulting in repeated, failed background event executions every 30 seconds, wasting resources.
Fix in Cursor Fix in Web

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Errors are already handled globally at the upper level, in src/index.ts. The try/catch was redundant.
  2. Wrong, we do schedule the next run after we've checked that the send form is open, which is the relevant condition that make sense to be checked.

Loading