Skip to content
This repository has been archived by the owner on Sep 23, 2024. It is now read-only.

Remove DB error details from the RPC handler response #124

Merged
merged 1 commit into from
Apr 16, 2024
Merged

Conversation

begmaroman
Copy link
Contributor

No description provided.

@begmaroman begmaroman requested a review from a team as a code owner April 15, 2024 14:33
@begmaroman begmaroman self-assigned this Apr 15, 2024
Copy link

return "0x0", jRPC.NewRPCError(jRPC.DefaultErrorCode, fmt.Sprintf("failed to execute tx: %s", err))
}

// Send L1 tx
dbTx, err := i.db.BeginStateTransaction(ctx)
if err != nil {
return "0x0", jRPC.NewRPCError(jRPC.DefaultErrorCode, fmt.Sprintf("failed to begin dbTx, error: %s", err))
log.Errorf("failed to begin dbTx, error: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

every time it returns here, will log an error, so this is redundant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not necessarily, it returns a short error without details but logs the full error message. It was the main idea of this PR.

return "0x0", jRPC.NewRPCError(jRPC.DefaultErrorCode, fmt.Sprintf("failed to commit dbTx, error: %s", err))

if err = dbTx.Commit(ctx); err != nil {
log.Errorf("failed to commit dbTx, error: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

every time it returns here, will log an error, so this is redundant?

@begmaroman begmaroman requested a review from vcastellm April 15, 2024 17:10
Copy link
Contributor

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

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

LGTM

@begmaroman begmaroman merged commit cd40b30 into main Apr 16, 2024
6 checks passed
@begmaroman begmaroman deleted the fix/CDK-182 branch April 16, 2024 09:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants