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

Add properties to describe failed requests #378

Closed
ghost opened this issue Apr 15, 2019 · 4 comments
Closed

Add properties to describe failed requests #378

ghost opened this issue Apr 15, 2019 · 4 comments
Labels
2.1.0-CSD.1 Will be fixed in SARIF v2.1.0 CSD.1. design-approved The TC approved the design and I can write the change draft enhancement impact-non-breaking-change merged Changes merged into provisional draft. resolved-fixed

Comments

@ghost
Copy link

ghost commented Apr 15, 2019

From @kupsch, regarding p.147, response object:

The response object should have a mechanism to indicate that no response was received from the server. This could happen due to a DNS failure or a connectivity issue such as the server not running or the host or port being inaccessible. Should add a requestFailed and requestFailureReason at a minimum. This is similar to an invocation where process creation failed.

@ghost ghost added 2.1.0-CSD.1 Will be fixed in SARIF v2.1.0 CSD.1. design-improvement labels Apr 15, 2019
@ghost ghost self-assigned this Apr 15, 2019
@ghost ghost mentioned this issue Apr 15, 2019
@michaelcfanning
Copy link
Contributor

TC approves this change:

request.noResponseReceived : defaults to false
request.failureReason : of type string

@ghost ghost changed the title Add response properties to describe failed requests Add properties to describe failed requests Apr 17, 2019
@ghost ghost added design-approved The TC approved the design and I can write the change draft enhancement impact-non-breaking-change to-be-written and removed design-improvement labels Apr 17, 2019
@michaelcfanning
Copy link
Contributor

@lgolding, on further review, I believe Jim had it right to begin with: noResponseReceived belongs on the response object. We should consider renaming 'reasonPhrase' to 'failureReason' to allow it to be used for this case. We should probably discuss in more detail soonest and loop in Jim.

@michaelcfanning
Copy link
Contributor

@lgolding these changes need to go into the schema asap

@ghost
Copy link
Author

ghost commented Apr 25, 2019

The current proposal is simply to add webResponse.noResponseReceived. We will overload webResponse.reasonPhrase to hold the error message in this case.

ghost pushed a commit that referenced this issue Apr 25, 2019
@ghost ghost added resolved-fixed and removed to-be-written labels Apr 25, 2019
@ghost ghost closed this as completed Apr 25, 2019
@ghost ghost added merged Changes merged into provisional draft. and removed schema-todo labels Apr 25, 2019
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.1.0-CSD.1 Will be fixed in SARIF v2.1.0 CSD.1. design-approved The TC approved the design and I can write the change draft enhancement impact-non-breaking-change merged Changes merged into provisional draft. resolved-fixed
Projects
None yet
Development

No branches or pull requests

1 participant