Skip to content

Conversation

@erezrokah
Copy link
Contributor

- Summary

Fixes #1227
Dependant on netlify/js-client#161

Handles the error thrown by js-client when there are no files to deploy (cancels the created deploy).

- Test plan

Will push a test once is netlify/js-client#161 merged and published

- Description for the changelog

Handle deploy error when there are no files to deploy

- A picture of a cute animal (not mandatory but encouraged)

🐳

@erezrokah erezrokah requested a review from ehmicky September 23, 2020 18:11
@github-actions github-actions bot added the type: bug code to address defects in shipped code label Sep 23, 2020
} catch (e) {
warn(`Failed canceling deploy with id ${deployId}: ${e.message}`)
}
await cancelDeploy({ api, deployId, warn })
Copy link
Contributor Author

@erezrokah erezrokah Sep 23, 2020

Choose a reason for hiding this comment

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

We can throw an error and let the calling function cancel the deploy, but we would need lift the spinner handling too.

filter: getDeployFilesFilter({ site, deployFolder }),
})
} catch (e) {
if (deployId) {
Copy link
Contributor

@ehmicky ehmicky Sep 23, 2020

Choose a reason for hiding this comment

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

[sand] Should this be put inside an additional try/catch inner block around deployEdgeHandlers() + api.deploy() instead? Like this, there would be no need to check if deployId exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would mean we need to re-throw the error after canceling the deploy to reach the other part of the error handling code (we could also refactor that part when we do netlify/js-client#157 to have a dedicated try/catch per operation to have better error reporting).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Going to merge and release. We can discuss this in netlify/js-client#157

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug code to address defects in shipped code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Manual deployment hangs with a deployment folder that starts with a dot

3 participants