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

[Enhancement] Substitute e.printStackTrace() with log.error() #4733

Closed
2 of 3 tasks
Pil0tXia opened this issue Jan 9, 2024 · 11 comments · Fixed by #4754
Closed
2 of 3 tasks

[Enhancement] Substitute e.printStackTrace() with log.error() #4733

Pil0tXia opened this issue Jan 9, 2024 · 11 comments · Fixed by #4754
Labels
enhancement New feature or request good first issue Issues for first-time contributors

Comments

@Pil0tXia
Copy link
Member

Pil0tXia commented Jan 9, 2024

Search before asking

  • I had searched in the issues and found no similar issues.

Enhancement Request

image

e.printStackTrace() should be substituted with log.error() in these classes:

  • org.apache.eventmesh.runtime.client.impl.PubClientImpl
  • org.apache.eventmesh.openconnect.SourceWorker
  • org.apache.eventmesh.common.ThreadWrapperTest

Describe the solution you'd like

Take org.apache.eventmesh.runtime.client.impl.PubClientImpl as an example:

image

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@Pil0tXia Pil0tXia added enhancement New feature or request good first issue Issues for first-time contributors labels Jan 9, 2024
@MovieTone
Copy link
Contributor

Hi @Pil0tXia, I can fix this enhancement issue as well, would be happy to be assigned

@Pil0tXia
Copy link
Member Author

Pil0tXia commented Jan 9, 2024

@MovieTone How about temporarily leaving this issue to someone else who is interested in contributing to open source?

@MovieTone
Copy link
Contributor

@Pil0tXia Yes, sure

@ppkarwasz
Copy link
Contributor

@Pil0tXia,

There is an OpenRewrite recipe for this: https://docs.openrewrite.org/recipes/java/logging/printstacktracetologerror

You can also run it regularly to convert new call sites of Throwable#printStackTrace() (some IDEs, such as Eclipse add it as default catch content).

@Pil0tXia
Copy link
Member Author

@ppkarwasz Hah, I remember that. There are only three remaining usages of printStackTrace(), which can be resolved in a good first issue.😉

@archit-8
Copy link

Hi @Pil0tXia, can you assign me this enhancement issue.

@Pil0tXia
Copy link
Member Author

@archit-8 Welcome to Apache EventMesh community!

@archit-8
Copy link

thanks, hey can you share path of a files where changes are going to happen.

@Pil0tXia
Copy link
Member Author

Pil0tXia commented Jan 14, 2024

@archit-8

share path of a files where changes are going to happen.

They are listed in the body of the issue already:

image

@mxsm
Copy link
Member

mxsm commented Jan 16, 2024

@Pil0tXia This issue has finished?

@Pil0tXia
Copy link
Member Author

@mxsm Not yet. The assignee haven't submit a PR yet.

archit-8 added a commit to archit-8/eventmesh that referenced this issue Jan 17, 2024
replaced e.printStackTrace() with log.error()
archit-8 added a commit to archit-8/eventmesh that referenced this issue Jan 17, 2024
replaced e.printStackTrace() with log.error()
Pil0tXia pushed a commit that referenced this issue Jan 21, 2024
* Substitute e.printStackTrace() with log.error()

* add exception arg

* Modify the wording of the log
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment