EVG-20983: Improve deploy script error handling and remove duplicate tag push #2108
Conversation
Passing run #13521 ↗︎
Details:
Review all test suite changes for PR #2108 ↗︎ |
||||||||||||||||||||
| console.log("Creating new tag..."); | ||
| createNewTag(); | ||
| console.log("Pushing tags..."); | ||
| pushTags(); |
There was a problem hiding this comment.
This is the duplicate tag push. createNewTag calls the postversion hook which runs scripts/push-version.sh
There was a problem hiding this comment.
I think there might be a space in the name of this file? Either that or the github UI is playing tricks.
| try { | ||
| execSync("yarn version --new-version patch", { | ||
| encoding: "utf-8", | ||
| stdio: "inherit", | ||
| }); | ||
| } catch (err) { | ||
| console.log("Creating new tag failed."); | ||
| console.log("output", err); | ||
| return false; | ||
| } | ||
| return true; |
There was a problem hiding this comment.
I think this composition is pretty confusing. Some util functions return these booleans indicating the status while others throw errors. What criteria is used to determine what should do what? Could we be a little more consistent in how we error?
How do you feel about treating errors the way we do in go and wrapping them?
https://stackoverflow.com/questions/42754270/re-throwing-exception-in-nodejs-and-not-losing-stack-trace
There was a problem hiding this comment.
Sweet, wrapping and re-throwing helps a lot.
There was a problem hiding this comment.
What criteria is used to determine what should do what? Could we be a little more consistent in how we error?
I wanted to throw and handle errors closer to the GH api functions so they are easier to track down and debug. Wrapping the error and including a message and then propagating that error to a top level try/catch is better though because 1) we get the benefit of adding help text to a specific code block and 2) have all of the errors collected in the same part of the program so the exit point is consolidated.
| const { value: shouldForceDeploy } = await prompts({ | ||
| type: "confirm", | ||
| name: "value", | ||
| message: `No new commits. Do you want to trigger a deploy on the most recent existing tag? (${latestTag})`, | ||
| initial: false, | ||
| }); |
There was a problem hiding this comment.
We ran into an issue last week when the deployer just hit enter and breezed through this message. Could we maybe have it default to false so that a user needs to be a little more careful to go through with this action?
There was a problem hiding this comment.
This is a good idea but I'll like to complete it in a separate ticket: https://jira.mongodb.org/browse/EVG-21115
| } catch (err) { | ||
| throw new Error("Creating new tag failed.", { cause: err }); | ||
| } | ||
| console.log("Pushed to remote. Should be deploying soon..."); |
There was a problem hiding this comment.
I moved logging to the helper function so the api request and followup messaging stay together. This also keeps the calling script clean.
EVG-20983
Adds error handling and exit codes to
evergreenDeploy,ciDeploy,localDeployand the postversion hook (push-version.sh) and removes a duplicate tag push when force deploying.