OfficeDev / Office-Addin-Scripts

A set of scripts and packages that are consumed in Office add-ins projects.
MIT License
156 stars 99 forks source link

office-addin-manifest returns success on non-existent manifest file path #856

Open tobz1000 opened 5 months ago

tobz1000 commented 5 months ago

Expected behavior

office-addin-manifest should return an error code when the specified manifest filepath is non-existent.

Current behavior

The tools returns a success code.

Steps to Reproduce

  1. Run office-addin-manifest validate not-a-dir/manifest.xml
  2. ENOENT error is printed, but the command's return code is 0.

Context

It looks like this issue has been previously raised and fixed (#372), so this may be a regression. EDIT: looks like that PR prints the appropriate error message, but doesn't change the exit code.

Failure Logs

$ npx office-addin-manifest validate not-a-dir/manifest.xml; echo $?
Error: Unable to read data for manifest file: not-a-dir/manifest.xml.
Error: ENOENT: no such file or directory, open 'not-a-dir/manifest.xml'
0
ndeleuze commented 5 months ago

@tobz1000 it is most likely that when we catch an error, we are just outputting the message and then gracefully completing rather than returning an error code. What is your scenario where you need the specific error code even when the output displays the error?

tobz1000 commented 5 months ago

Since the command is for validating the manifest file, it would make sense to return an error if the file can't be found at all. I use the command in a linting script for my codebase. If the manifest filepath is incorrect, the script returns success, suggesting no errors; but it should fail, as the manifest possibly doesn't even exist.

ndeleuze commented 5 months ago

Thanks for reporting this issue regarding office-addin-manifest validate script not returning the proper error code for this case. It has been put on our backlog. We unfortunately have no timelines to share at this point. However, I would suggest as a workaround that you validate the existence of the manifest before using the office-addin-manifest validate script.

Internal tracking id: Office: 8958771