Skip to content

Conversation

@mengxpgogogo-eng
Copy link
Contributor

This commit improves code quality by replacing printStackTrace() calls with proper SLF4J logging in connector modules.

Changes:

  • GraphQLWebSocket: Replaced 3 printStackTrace() calls with log.error()
  • ActivemqClient: Replaced 1 printStackTrace() call with log.error()
  • SlsSinkWriter: Replaced 1 printStackTrace() call with log.error()

Additional improvements:

  • Added proper interrupt handling for InterruptedException
  • Ensured exception stack traces are properly logged via SLF4J

Benefits:

  1. Better production logging - stack traces go to log files instead of stderr
  2. Consistent with project logging standards
  3. Easier to debug issues in production environments
  4. Follows Java best practices for exception handling

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

This commit improves code quality by replacing printStackTrace() calls
with proper SLF4J logging in connector modules.

Changes:
- GraphQLWebSocket: Replaced 3 printStackTrace() calls with log.error()
- ActivemqClient: Replaced 1 printStackTrace() call with log.error()
- SlsSinkWriter: Replaced 1 printStackTrace() call with log.error()

Additional improvements:
- Added proper interrupt handling for InterruptedException
- Ensured exception stack traces are properly logged via SLF4J

Benefits:
1. Better production logging - stack traces go to log files instead of stderr
2. Consistent with project logging standards
3. Easier to debug issues in production environments
4. Follows Java best practices for exception handling
Copy link
Contributor

@chl-wxp chl-wxp left a comment

Choose a reason for hiding this comment

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

LGTM

davidzollo
davidzollo previously approved these changes Dec 16, 2025
Copy link
Contributor

@davidzollo davidzollo left a comment

Choose a reason for hiding this comment

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

+1
Good job

…ache/seatunnel/connectors/seatunnel/graphql/source/reader/GraphQLWebSocket.java

Co-authored-by: dy102 <[email protected]>
davidzollo
davidzollo previously approved these changes Dec 16, 2025
Copy link
Contributor

@davidzollo davidzollo left a comment

Choose a reason for hiding this comment

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

+1
Good job

Copy link
Member

@zhangshenghang zhangshenghang left a comment

Choose a reason for hiding this comment

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

don't delete .idea/vcs.xml

@mengxpgogogo-eng
Copy link
Contributor Author

don't delete .idea/vcs.xml

Thanks I have restored .idea/vcs.xml file

Copy link
Contributor

@davidzollo davidzollo left a comment

Choose a reason for hiding this comment

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

+1

@zhangshenghang zhangshenghang merged commit 86c8450 into apache:dev Dec 17, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants