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.

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

Add AppError to Sender interface #2737

Merged
merged 5 commits into from
Feb 19, 2024
Merged

Add AppError to Sender interface #2737

merged 5 commits into from
Feb 19, 2024

Conversation

joshua-kim
Copy link
Contributor

@joshua-kim joshua-kim commented Feb 15, 2024

Why this should be merged

Adds SendAppError* functions to the sender interface. Handling for this message already exists but we didn't expose a way for a VM to send these messages previously.

How this works

Exposes a new function to send app errors

How this was tested

Tested manually locally

[02-15|13:12:20.637] DEBUG <X Chain> gossip/gossip.go:172 failed gossip request {"nodeID": "NodeID-MFrZFVCXPv5iCn6M9K6XduxGTYp891xXZ", "error": "-123: random error"}

Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
@joshua-kim joshua-kim marked this pull request as ready for review February 15, 2024 18:16
@joshua-kim joshua-kim added the networking This involves networking label Feb 15, 2024
Copy link
Collaborator

@aaronbuchwald aaronbuchwald left a comment

Choose a reason for hiding this comment

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

LGTM - first thought was we should add tests for this, but since there are currently no tests for the sender path, I think it's better not to couple that to this PR for now.

snow/engine/common/sender.go Outdated Show resolved Hide resolved
Signed-off-by: Stephen Buttolph <stephen@avalabs.org>
@StephenButtolph StephenButtolph added this to the v1.11.0 milestone Feb 19, 2024
@StephenButtolph StephenButtolph changed the title App error sender Add AppError to Sender interface Feb 19, 2024
@StephenButtolph StephenButtolph added this pull request to the merge queue Feb 19, 2024
Merged via the queue into master with commit 908484b Feb 19, 2024
17 checks passed
@StephenButtolph StephenButtolph deleted the app-error-sender branch February 19, 2024 17:05
mboben pushed a commit to mboben/avalanchego that referenced this pull request Apr 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
networking This involves networking
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants