core.setFailed and core.error to include stack trace to make debugging and triaging easier

[iamstarkov]

currently its impossible to know which place in the code caused an error if you have simple syntax errors like replace is not a function.

and if I get an report of an error with my action, report won't have anything meaningful to mention, which makes triaging process few steps longer than needed

[iamstarkov]

@robherley@Link- can you take a look?

[iamstarkov]

@GhadimiR can you take a look?

[GhadimiR]

Hey @iamstarkov, thanks for contributing! I recognise the pain point you're trying to address here with the use of toString() over stack, and can see how the proposed change would be beneficial to your workflow.

Unfortunately, as this is likely to be a breaking change for some of our users, we can't adopt this change right now, however I've opened an issue with a feature request for this and hopefully in the future we can find the time to get this implemented safely.

Thanks!

[iamstarkov]

@GhadimiR thank you

[iamstarkov]

@GhadimiR what if we introduce this change as new entry to the api surface? perhaps core.setFailedWithStack? this won't be the breaking change anymore. if its acceptable I can change that and adjust docs to go with it

[iamstarkov]

and while you are at it, can you take a look at another PR of mine #1926

[GhadimiR]

Hey @iamstarkov, we've had a discussion on this internally and our position on this hasn't changed. If we're increasing the API surface as opposed to changing the default behaviour, this achieves the same thing as the caller just using core.error(error.stack). We will keep the issue open and may choose to revisit the default behaviour in a future major release.

[iamstarkov]

Alright, I see. Thank you for your time

[iamstarkov]

currently its impossible to know which place in the code caused an error if you have simple syntax errors like replace is not a function.

and if I get an report of an error with my action, report won't have anything meaningful to mention, which makes triaging process few steps longer than needed

[iamstarkov]

@robherley@Link- can you take a look?

[iamstarkov]

@GhadimiR can you take a look?

[GhadimiR]

Hey @iamstarkov, thanks for contributing! I recognise the pain point you're trying to address here with the use of toString() over stack, and can see how the proposed change would be beneficial to your workflow.

Unfortunately, as this is likely to be a breaking change for some of our users, we can't adopt this change right now, however I've opened an issue with a feature request for this and hopefully in the future we can find the time to get this implemented safely.

Thanks!

[iamstarkov]

@GhadimiR thank you

[iamstarkov]

@GhadimiR what if we introduce this change as new entry to the api surface? perhaps core.setFailedWithStack? this won't be the breaking change anymore. if its acceptable I can change that and adjust docs to go with it

[iamstarkov]

and while you are at it, can you take a look at another PR of mine #1926

[GhadimiR]

Hey @iamstarkov, we've had a discussion on this internally and our position on this hasn't changed. If we're increasing the API surface as opposed to changing the default behaviour, this achieves the same thing as the caller just using core.error(error.stack). We will keep the issue open and may choose to revisit the default behaviour in a future major release.

[iamstarkov]

Alright, I see. Thank you for your time

[iamstarkov]

currently its impossible to know which place in the code caused an error if you have simple syntax errors like replace is not a function.

and if I get an report of an error with my action, report won't have anything meaningful to mention, which makes triaging process few steps longer than needed

[iamstarkov]

@robherley@Link- can you take a look?

[iamstarkov]

@GhadimiR can you take a look?

[GhadimiR]

Hey @iamstarkov, thanks for contributing! I recognise the pain point you're trying to address here with the use of toString() over stack, and can see how the proposed change would be beneficial to your workflow.

Unfortunately, as this is likely to be a breaking change for some of our users, we can't adopt this change right now, however I've opened an issue with a feature request for this and hopefully in the future we can find the time to get this implemented safely.

Thanks!

[iamstarkov]

@GhadimiR thank you

[iamstarkov]

@GhadimiR what if we introduce this change as new entry to the api surface? perhaps core.setFailedWithStack? this won't be the breaking change anymore. if its acceptable I can change that and adjust docs to go with it

[iamstarkov]

and while you are at it, can you take a look at another PR of mine #1926

[GhadimiR]

Hey @iamstarkov, we've had a discussion on this internally and our position on this hasn't changed. If we're increasing the API surface as opposed to changing the default behaviour, this achieves the same thing as the caller just using core.error(error.stack). We will keep the issue open and may choose to revisit the default behaviour in a future major release.

[iamstarkov]

Alright, I see. Thank you for your time

[iamstarkov]

currently its impossible to know which place in the code caused an error if you have simple syntax errors like replace is not a function.

and if I get an report of an error with my action, report won't have anything meaningful to mention, which makes triaging process few steps longer than needed

[iamstarkov]

@robherley@Link- can you take a look?

[iamstarkov]

@GhadimiR can you take a look?

[GhadimiR]

Hey @iamstarkov, thanks for contributing! I recognise the pain point you're trying to address here with the use of toString() over stack, and can see how the proposed change would be beneficial to your workflow.

Unfortunately, as this is likely to be a breaking change for some of our users, we can't adopt this change right now, however I've opened an issue with a feature request for this and hopefully in the future we can find the time to get this implemented safely.

Thanks!

[iamstarkov]

@GhadimiR thank you

[iamstarkov]

@GhadimiR what if we introduce this change as new entry to the api surface? perhaps core.setFailedWithStack? this won't be the breaking change anymore. if its acceptable I can change that and adjust docs to go with it

[iamstarkov]

and while you are at it, can you take a look at another PR of mine #1926

[GhadimiR]

Hey @iamstarkov, we've had a discussion on this internally and our position on this hasn't changed. If we're increasing the API surface as opposed to changing the default behaviour, this achieves the same thing as the caller just using core.error(error.stack). We will keep the issue open and may choose to revisit the default behaviour in a future major release.

[iamstarkov]

Alright, I see. Thank you for your time

[iamstarkov]

currently its impossible to know which place in the code caused an error if you have simple syntax errors like replace is not a function.

and if I get an report of an error with my action, report won't have anything meaningful to mention, which makes triaging process few steps longer than needed

[iamstarkov]

@robherley@Link- can you take a look?

[iamstarkov]

@GhadimiR can you take a look?

[GhadimiR]

Hey @iamstarkov, thanks for contributing! I recognise the pain point you're trying to address here with the use of toString() over stack, and can see how the proposed change would be beneficial to your workflow.

Unfortunately, as this is likely to be a breaking change for some of our users, we can't adopt this change right now, however I've opened an issue with a feature request for this and hopefully in the future we can find the time to get this implemented safely.

Thanks!

[iamstarkov]

@GhadimiR thank you

[iamstarkov]

@GhadimiR what if we introduce this change as new entry to the api surface? perhaps core.setFailedWithStack? this won't be the breaking change anymore. if its acceptable I can change that and adjust docs to go with it

[iamstarkov]

and while you are at it, can you take a look at another PR of mine #1926

[GhadimiR]

Hey @iamstarkov, we've had a discussion on this internally and our position on this hasn't changed. If we're increasing the API surface as opposed to changing the default behaviour, this achieves the same thing as the caller just using core.error(error.stack). We will keep the issue open and may choose to revisit the default behaviour in a future major release.

[iamstarkov]

Alright, I see. Thank you for your time

[iamstarkov]

currently its impossible to know which place in the code caused an error if you have simple syntax errors like replace is not a function.

and if I get an report of an error with my action, report won't have anything meaningful to mention, which makes triaging process few steps longer than needed

[iamstarkov]

@robherley@Link- can you take a look?

[iamstarkov]

@GhadimiR can you take a look?

[GhadimiR]

Hey @iamstarkov, thanks for contributing! I recognise the pain point you're trying to address here with the use of toString() over stack, and can see how the proposed change would be beneficial to your workflow.

Unfortunately, as this is likely to be a breaking change for some of our users, we can't adopt this change right now, however I've opened an issue with a feature request for this and hopefully in the future we can find the time to get this implemented safely.

Thanks!

[iamstarkov]

@GhadimiR thank you

[iamstarkov]

@GhadimiR what if we introduce this change as new entry to the api surface? perhaps core.setFailedWithStack? this won't be the breaking change anymore. if its acceptable I can change that and adjust docs to go with it

[iamstarkov]

and while you are at it, can you take a look at another PR of mine #1926

[GhadimiR]

Hey @iamstarkov, we've had a discussion on this internally and our position on this hasn't changed. If we're increasing the API surface as opposed to changing the default behaviour, this achieves the same thing as the caller just using core.error(error.stack). We will keep the issue open and may choose to revisit the default behaviour in a future major release.

[iamstarkov]

Alright, I see. Thank you for your time

[iamstarkov]

currently its impossible to know which place in the code caused an error if you have simple syntax errors like replace is not a function.

and if I get an report of an error with my action, report won't have anything meaningful to mention, which makes triaging process few steps longer than needed

[iamstarkov]

@robherley@Link- can you take a look?

[iamstarkov]

@GhadimiR can you take a look?

[GhadimiR]

Hey @iamstarkov, thanks for contributing! I recognise the pain point you're trying to address here with the use of toString() over stack, and can see how the proposed change would be beneficial to your workflow.

Unfortunately, as this is likely to be a breaking change for some of our users, we can't adopt this change right now, however I've opened an issue with a feature request for this and hopefully in the future we can find the time to get this implemented safely.

Thanks!

[iamstarkov]

@GhadimiR thank you

[iamstarkov]

@GhadimiR what if we introduce this change as new entry to the api surface? perhaps core.setFailedWithStack? this won't be the breaking change anymore. if its acceptable I can change that and adjust docs to go with it

[iamstarkov]

and while you are at it, can you take a look at another PR of mine #1926

[GhadimiR]

Hey @iamstarkov, we've had a discussion on this internally and our position on this hasn't changed. If we're increasing the API surface as opposed to changing the default behaviour, this achieves the same thing as the caller just using core.error(error.stack). We will keep the issue open and may choose to revisit the default behaviour in a future major release.

[iamstarkov]

Alright, I see. Thank you for your time

[iamstarkov]

currently its impossible to know which place in the code caused an error if you have simple syntax errors like replace is not a function.

and if I get an report of an error with my action, report won't have anything meaningful to mention, which makes triaging process few steps longer than needed

[iamstarkov]

@robherley@Link- can you take a look?

[iamstarkov]

@GhadimiR can you take a look?

[GhadimiR]

Hey @iamstarkov, thanks for contributing! I recognise the pain point you're trying to address here with the use of toString() over stack, and can see how the proposed change would be beneficial to your workflow.

Unfortunately, as this is likely to be a breaking change for some of our users, we can't adopt this change right now, however I've opened an issue with a feature request for this and hopefully in the future we can find the time to get this implemented safely.

Thanks!

[iamstarkov]

@GhadimiR thank you

[iamstarkov]

@GhadimiR what if we introduce this change as new entry to the api surface? perhaps core.setFailedWithStack? this won't be the breaking change anymore. if its acceptable I can change that and adjust docs to go with it

[iamstarkov]

and while you are at it, can you take a look at another PR of mine #1926

[GhadimiR]

Hey @iamstarkov, we've had a discussion on this internally and our position on this hasn't changed. If we're increasing the API surface as opposed to changing the default behaviour, this achieves the same thing as the caller just using core.error(error.stack). We will keep the issue open and may choose to revisit the default behaviour in a future major release.

[iamstarkov]

Alright, I see. Thank you for your time