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

fix: panic with normal errors in Rust #1377

Merged
merged 1 commit into from
Apr 16, 2024
Merged

Conversation

satoren
Copy link
Contributor

@satoren satoren commented Apr 16, 2024

Returns an error when an error can be returned

Copy link
Collaborator

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

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

Did you actually hit this? If so then under which condition?
This is the kind of error that we indeed do not expect unless I missed something.

@satoren
Copy link
Contributor Author

satoren commented Apr 16, 2024

It is defined as a RequestError. Seems to be returning it except here. The type is also Result<ConsumerStats, RequestError>.

pub enum RequestError {

@nazar-pc
Copy link
Collaborator

IIRC ConsumerGetStatsRequest never errors, it is just ignored if consumer doesn't exist. No strong opinion though.

@ibc
Copy link
Member

ibc commented Apr 16, 2024

So should we merge or not?

Copy link
Collaborator

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

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

Sure, I don't think it will ever happen, but it doesn't hurt either

@satoren
Copy link
Contributor Author

satoren commented Apr 16, 2024

I have encountered the L1042 panic, but I don't know up to the exact cause because there is no other information available.

Maybe it is an error that the "Channel already closed".

@ibc ibc merged commit d6373d5 into versatica:v3 Apr 16, 2024
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants