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 error code for protocol error #2259

Closed

Conversation

KunZhou-at
Copy link
Contributor

The error code emitted for ER_CLIENT_INTERACTION_TIMEOUT is "4031" instead of the expected "ER_CLIENT_INTERACTION_TIMEOUT", this fixes that.

@KunZhou-at KunZhou-at force-pushed the fixErrorCodeForProtocolError branch from fb82aba to e2d503a Compare October 25, 2023 20:46
@KunZhou-at KunZhou-at force-pushed the fixErrorCodeForProtocolError branch from e2d503a to e047956 Compare October 25, 2023 21:24
@KunZhou-at
Copy link
Contributor Author

cc @wellwelwel bump on this

@wellwelwel wellwelwel mentioned this pull request Apr 17, 2024
@wellwelwel
Copy link
Collaborator

cc @wellwelwel bump on this

@KunZhou-at, thank you for your many contributions 💙

This one in particular I couldn't figure out what the issue was and what the solution does. Could you please show a failure that occurs without your fix?

More personally, when I see that no tests have been changed and a new test covers a failure, I usually merge the PRs. When I don't understand or I realize a decision would need to be taken, I usually leave it to @sidorares and also to the community 🤝

@KunZhou-at
Copy link
Contributor Author

KunZhou-at commented Apr 17, 2024

@wellwelwel I believe error.code is a number in the current code for ER_CLIENT_INTERACTION_TIMEOUT but we have always propagated the error.code to the client as a string elsewhere (for other errors). So this seems to be inconsistent behavior.

@KunZhou-at
Copy link
Contributor Author

@wellwelwel Closing since it's not getting attention. Feel free to open and I can resolve the conflicts if that changes.

@KunZhou-at KunZhou-at closed this May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants