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

Jump to bottom

feat: log transactions that gets no receipt #2048

Merged
merged 2 commits into from
Nov 7, 2024
Merged

feat: log transactions that gets no receipt #2048

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1bf782f
Log transaction that gets no receipt
edisontim Nov 7, 2024
16e679d
Change error message
edisontim Nov 7, 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
12 changes: 9 additions & 3 deletions sdk/packages/eternum/src/provider/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,15 @@ export class EternumProvider extends EnhancedDojoProvider {

// Wrapper function to check for transaction errors
async waitForTransactionWithCheck(transactionHash: string) {
const receipt = await this.provider.waitForTransaction(transactionHash, {
retryInterval: 500,
});
let receipt;
try {
receipt = await this.provider.waitForTransaction(transactionHash, {
retryInterval: 500,
});
} catch (error) {
console.error(error);
throw error;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance error handling and logging.

The current implementation has several areas for improvement:

  1. Replace console.error with structured logging that includes the transaction hash for better traceability.
  2. Preserve error context when re-throwing.
  3. Emit an event for consistency with other error cases.

Consider applying this improvement:

 let receipt;
 try {
   receipt = await this.provider.waitForTransaction(transactionHash, {
     retryInterval: 500,
   });
 } catch (error) {
-  console.error(error);
-  throw error;
+  const enhancedError = new Error(`Failed to wait for transaction ${transactionHash}: ${error.message}`);
+  enhancedError.cause = error;
+  this.emit("transactionError", { transactionHash, error: enhancedError });
+  throw enhancedError;
 }
📝 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
let receipt;
try {
receipt = await this.provider.waitForTransaction(transactionHash, {
retryInterval: 500,
});
} catch (error) {
console.error(error);
throw error;
}
let receipt;
try {
receipt = await this.provider.waitForTransaction(transactionHash, {
retryInterval: 500,
});
} catch (error) {
const enhancedError = new Error(`Failed to wait for transaction ${transactionHash}: ${error.message}`);
enhancedError.cause = error;
this.emit("transactionError", { transactionHash, error: enhancedError });
throw enhancedError;
}


// Check if the transaction was reverted and throw an error if it was
if (receipt.isReverted()) {
Expand Down
Loading