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

Provide a more helpful TimeoutException message #12538

Conversation

garydgregory
Copy link
Contributor

@garydgregory garydgregory commented Nov 14, 2024

  • One cannot tell what endpoint is at fault in a non-trivial configuration, for example, using an AsyncMiddleManServlet, is it the origin or the client?
  • The TimeoutException message now contains the endpoint causing the timeout

- One cannot tell what endpoint is at fault in a non-trivial
configuration, for example, using an AsyncMiddleManServlet, is it the
origin or the client?
- The TimeoutException message now contains the endpoint causing the
timeout
@sbordet
Copy link
Contributor

sbordet commented Nov 14, 2024

@garydgregory I have doubts that we can accept this, as we need to be very careful about exposing IP addresses (even internal ones) in exception messages that may be seen by remote users.

In AsyncMiddleManServlet you have onClientRequestFailure() and onProxyResponseFailure() to determine whether it is the client or the server.

@joakime @gregw @lorban opinions about disclosing the IP in the exception?

@sbordet sbordet self-requested a review November 14, 2024 15:53
@garydgregory
Copy link
Contributor Author

Hi all,

In the failure methods, I cannot tell from the throwable which endpoint threw the exception.

Would you rather the TimeoutException point to the endpoint that created it? This could only be done in a new custom TimeoutException subclass.

@sbordet
Copy link
Contributor

sbordet commented Nov 14, 2024

@garydgregory the failure methods have the request objects, and you can call e.g. HttpServletRequest.getRemoteAddr().
Did you try and it's not working?

@garydgregory
Copy link
Contributor Author

@garydgregory the failure methods have the request objects, and you can call e.g. HttpServletRequest.getRemoteAddr(). Did you try and it's not working?

Hello @sbordet

Thank you for your suggestion. I'll try using the 'remote' methods in our servlet and see what get in our logs... TY!

@garydgregory
Copy link
Contributor Author

FYI I still have some testing to do and reviewing of our logs.

@sbordet
Copy link
Contributor

sbordet commented Dec 10, 2024

@garydgregory do you have an update? I'm inclined to close this, as the "remote" methods should be enough for your use case?

@garydgregory
Copy link
Contributor Author

@sbordet
Yes, sorry about the delay. I'll close this.

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