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.

Sign up for GitHub

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

Jump to bottom

Do not swallow original excpetion when we have trouble to cat the logs #614

Merged
merged 4 commits into from
Apr 12, 2019
Merged

Do not swallow original excpetion when we have trouble to cat the logs #614

merged 4 commits into from
Apr 12, 2019
+62 −1

Conversation

marcusholl
Copy link
Member

relates to this issue

@marcusholl
Copy link
Member Author

In case there is an exception with the neoDeploy or also before invoking neo we end up in an exception handler. While handling the exception we try to cat the neo logs into the job log file. In case this fails we swallow the original exception. Hence we cannot see the reason for the failure anymore. Such situations increases the support load.

@marcusholl
Copy link
Member Author

Fix verified in productive project (inhouse).

This was referenced Apr 3, 2019
Closed
Closed
@marcusholl
Copy link
Member Author

@OliverNocon , @daniel-kurzynski , @CCFenner : This PR is open for almost two weeks. Your review is very much appreciated.

@marcusholl marcusholl added the REVIEW Pull-Request needs a review label Apr 11, 2019
daniel-kurzynski
daniel-kurzynski reviewed Apr 12, 2019
View reviewed changes
vars/neoDeploy.groovy Outdated
sh "cat logs/neo/*"
} catch(Exception e) {
echo "Unable to provide the logs."
suppressed = e
Copy link
Member

Choose a reason for hiding this comment

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

Why and not here: ex.addSuppressed(e)

Copy link
Member Author

Choose a reason for hiding this comment

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

@daniel-kurzynski : thanks for your hint. Your approach is much more straight-forward. I guess my mind was trapped in some troubleshooting mode while writing that code. I changed the code like suggested.

@marcusholl marcusholl force-pushed the pr/doNotSwallowOriginalException branch from 6d37118 to 28fa30f Compare April 12, 2019 08:06
@marcusholl
Adjust test after merging early check for the deployable
862729a 
In the meantime we check earlier if the deployable exists. Up to now the test used that late check. Now, as that
check happens earlier in the code we have to tune here. Now we assume a failure inside neo deploy instead.
@marcusholl
Merge remote-tracking branch 'github/master' into HEAD
fc93326
@marcusholl marcusholl force-pushed the pr/doNotSwallowOriginalException branch from 1b11713 to fc93326 Compare April 12, 2019 11:13
@marcusholl marcusholl merged commit bf267b7 into SAP:master Apr 12, 2019
@marcusholl marcusholl removed the REVIEW Pull-Request needs a review label May 13, 2019
@marcusholl marcusholl deleted the pr/doNotSwallowOriginalException branch October 22, 2020 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daniel-kurzynski daniel-kurzynski daniel-kurzynski left review comments

@OliverNocon OliverNocon OliverNocon approved these changes

@CCFenner CCFenner Awaiting requested review from CCFenner

Assignees
No one assigned
Labels
bug 🐛
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@marcusholl @daniel-kurzynski @OliverNocon