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

chore: update panic error handling in containers #173

Merged
merged 10 commits into from
Feb 4, 2025
Merged

Conversation

adarsh0728
Copy link
Contributor

@adarsh0728 adarsh0728 commented Jan 25, 2025

  1. Adds prefix UDF_EXECUTION_ERROR in panic mssgs in all containers to use it across SDK's to formalize error from user's code. This will help in debuggability and distinguishing between error in platform and user code.

  2. Fixed error handling in case of sideInput, sourceTransformer

  3. Error handling In case of reducer, sessionReducer, reduceStreamer (we are not sending error back in case of panics) to be taken up in separate PR.

  4. Added debug stack in details part which represents a serialized google.rpc.Status, the bytes should be parsed as a Protobuf message at client side.

fixes #174

Copy link
Member

@KeranYang KeranYang left a comment

Choose a reason for hiding this comment

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

Thanks for making the change Adarsh. Could you fix the unit test?

@adarsh0728 adarsh0728 marked this pull request as draft January 27, 2025 02:21
@adarsh0728
Copy link
Contributor Author

Thanks for making the change Adarsh. Could you fix the unit test?

yes! forgot to convert it into draft :)

@adarsh0728 adarsh0728 requested a review from yhl25 January 27, 2025 05:38
@adarsh0728 adarsh0728 changed the title chore: update error handling to include panic error message in source transformer chore: update error handling in source transformer Jan 27, 2025
@adarsh0728 adarsh0728 linked an issue Jan 27, 2025 that may be closed by this pull request
@adarsh0728 adarsh0728 changed the title chore: update error handling in source transformer chore: update panic error handling in containers Jan 28, 2025
@adarsh0728 adarsh0728 marked this pull request as ready for review January 29, 2025 11:14
@adarsh0728 adarsh0728 requested a review from KeranYang January 29, 2025 11:14
@yhl25
Copy link
Contributor

yhl25 commented Jan 30, 2025

Please return the stack trace along with the error message, in future when we want to show errors in the UI, stack traces will help.

@adarsh0728 adarsh0728 requested a review from whynowy January 30, 2025 04:21
Signed-off-by: adarsh0728 <[email protected]>
Signed-off-by: adarsh0728 <[email protected]>
@adarsh0728 adarsh0728 requested review from yhl25 and removed request for yhl25 and whynowy January 31, 2025 11:44
@yhl25 yhl25 merged commit 1d29855 into main Feb 4, 2025
3 checks passed
@yhl25 yhl25 deleted the err-transformer branch February 4, 2025 03:38
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.

Add common prefix and formalize error messages
4 participants