-
Notifications
You must be signed in to change notification settings - Fork 551
Move watch callback out of try-catch #857
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
Conversation
|
Welcome @Nicklason! |
Thanks for the PR! Code looks good. Can you add a unit test to cover this behavior? You should be able to adapt the code from here: https://github.com/kubernetes-client/javascript/blob/master/src/watch_test.ts#L320 but remove the parse error and set your watch handler to throw. |
@brendandburns I have added the test. It tests if the handler receives the correct values and if the promise fails because of the handler error without calling the done callback. |
/lgtm Thanks! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brendandburns, Nicklason The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Issue: #856