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.

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

Suggested SARIF File Merge Failure Improvements #2847

Open
davewichers opened this issue Jan 10, 2025 · 0 comments
Open

Suggested SARIF File Merge Failure Improvements #2847

davewichers opened this issue Jan 10, 2025 · 0 comments

Comments

@davewichers
Copy link

I'm doing a merge like this: $ npx @microsoft/sarif-multitool merge ToolResultsPart*.sarif --output-file ToolResultsALL.sarif

There are only 2 files being merged: ToolResultsPart1.sarif and ToolResultsPart2.sarif

And this is the output I get:
Required property 'text' not found in JSON. Path 'runs[0].results[0].codeFlows[0].threadFlows[0].locations[0].location.physicalLocation.region.snippet.rendered', line 80, position 44.
Required property 'text' not found in JSON. Path 'runs[0].results[0].codeFlows[0].threadFlows[0].locations[0].location.physicalLocation.region.snippet.rendered', line 101, position 44.
Merge completed in 00:00:26.7811596.

There are a number of problems/improvements that could be made here:

  1. Before listing what is wrong, can you add a line before all the errors in that file, that shows the name of the file first, so you know which file it is referring to?
  2. It says, 'Merge completed', but the merge actually failed. A 132 byte ToolRestulsALL.sarif file was created, which only contains this:

{
"$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.6.json",
"version": "2.1.0",
"runs": []
}

Seems like the last line of output in this situation should be more like: "Merge failed due to SARIF file errors reported above (in 00:00:26.7811596)."

  1. Is there a way to force the merge, even with these errors? I'd prefer that the tool do a best effort, rather than simply abort. It's OK if the default is to fail, but it would be nice if there was an option, to continue on.

Per these suggestions, the default output could look something like this:

Error(s) Detected in merge file: ToolResultsPart1.sarif:
Required property 'text' not found in JSON. Path 'runs[0].results[0].codeFlows[0].threadFlows[0].locations[0].location.physicalLocation.region.snippet.rendered', line 80, position 44.
Required property 'text' not found in JSON. Path 'runs[0].results[0].codeFlows[0].threadFlows[0].locations[0].location.physicalLocation.region.snippet.rendered', line 101, position 44.
Merge failed due to SARIF file errors reported above (in 00:00:26.7811596).

If you are able/willing to implement a 'best effort' merge, maybe it would look something like:

npx @microsoft/sarif-multitool merge ToolResultsPart*.sarif --output-file ToolResultsALL.sarif --continue-on-sarif-format-error

Error(s) Detected in merge file: ToolResultsPart1.sarif:
Required property 'text' not found in JSON. Path 'runs[0].results[0].codeFlows[0].threadFlows[0].locations[0].location.physicalLocation.region.snippet.rendered', line 80, position 44.
Required property 'text' not found in JSON. Path 'runs[0].results[0].codeFlows[0].threadFlows[0].locations[0].location.physicalLocation.region.snippet.rendered', line 101, position 44.
Error(s) Detected in merge file: ToolResultsPart2.sarif:
-- list of errors in this file.
Best Effort Merge completed in 00:TIME_ELAPSED
WARNING: Merged SARIF file might have issues due to 'best effort' actions taken during merge.

If there are no errors detected, then it should output the normal completion message. i.e.,

Merge completed in 00:TIME_ELAPSED

And it's perfectly reasonable to have some situations you can't try to autorecover from, but for minor issues like the above, it wouldn't be hard. In the above example, you could simply add the missing text element, and leave it blank.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant