Mark limits errors from third-party SARIF uploads as configuration errors #2173
Conversation
Nitty: make it a little clearer when this shows up in the logs what type of request we mean
| actionsUtil.getRequiredInput("checkout_path"), | ||
| actionsUtil.getOptionalInput("category"), | ||
| logger, | ||
| { isThirdPartyUpload: false }, |
There was a problem hiding this comment.
I'm not sure I understand why we can remove isThirdPartyUpload here: wouldn't this mean that we are also marking first-party upload failures as configuration errors?
There was a problem hiding this comment.
I've moved the logic where we convert InvalidSarifUploadErrors to ConfigurationErrors to upload-sarif-action.ts, which is the only place where we had isThirdPartyUpload: true. So where we had { isThirdPartyUpload: false }, we will not convert the InvalidSarifUploadErrors to ConfigurationErrors.
There was a problem hiding this comment.
I've also pushed a couple of commits so that we don't convert InvalidSarifUploadErrors to ConfigurationErrors when we're a first-party analysis as determined by our existing status report functionality.
angelapwen
left a comment
angelapwen
left a comment
There was a problem hiding this comment.
Looks good to me 👍 after this is merged I'll kick off an Action release to get this + the SyntaxError changes in.
henrymercer
deleted the
henrymercer/mark-third-party-limits-errors-as-configuration-errors
branch
I think this was the intention of the original PR, but the place that we were converting
InvalidSarifUploadErrors toConfigurationErrorswasn't including processing errors.Merge / deployment checklist