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

ZOOKEEPER-4182: Enhance message when packet's length over limit. #1580

Closed
wants to merge 1 commit into from

Conversation

horizonzy
Copy link
Member

@horizonzy horizonzy commented Jan 18, 2021

Append packetLen limit info to the exception message:

  • Tell users the default packet length limit and what value(jute.maxBuffer) they had set
  • For better debugging

@maoling maoling self-requested a review January 22, 2021 12:43
@maoling
Copy link
Member

maoling commented Feb 17, 2021

@horizonzy looks good.Thanks for your contribution.
Could you please add more to the GitHub description about what's the intention or advantage the change will bring.

@horizonzy
Copy link
Member Author

@horizonzy looks good.Thanks for your contribution.
Could you please add more to the GitHub description about what's the intention or advantage the change will bring.

Tell user the default packge length limit, In my opinion, the message can even tell user the way to modify the default limit. I will append a commit.

@horizonzy
Copy link
Member Author

what do you think of it

@maoling
Copy link
Member

maoling commented Feb 18, 2021

@horizonzy

Tell user the default packge length limit, In my opinion, the message can even tell user the way to modify the default limit. I will append a commit.

Looks good:)

@maoling
Copy link
Member

maoling commented Feb 21, 2021

@horizonzy

  • throw new IOException("Packet len " + len + " is out of range " + packetLen + "!"); is good enough, adding the operation guideline -Djute.maxBuffer to cover it. to the exception message is a little toilsome
  • The only thing I suggest is: give this PR a description. I do it for you:)

@horizonzy
Copy link
Member Author

horizonzy commented Feb 21, 2021

@horizonzy

  • throw new IOException("Packet len " + len + " is out of range " + packetLen + "!"); is good enough, adding the operation guideline -Djute.maxBuffer to cover it. to the exception message is a little toilsome
  • The only thing I suggest is: give this PR a description. I do it for you:)

Alright. Thank you :)

@ztzg
Copy link
Contributor

ztzg commented Feb 25, 2021

Thank you, @horizonzy. I'm afraid I'm going to close this in favor of #1614, however, as the message is even more informative.

@ztzg ztzg closed this Feb 25, 2021
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.

3 participants