Add support for better API debugging; start v1.5.0 development #325
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
theckman
force-pushed
the
better_last_response_handling
branch
from
2941d9c
to
e27c9e5
Compare
theckman
commented
May 8, 2021
| ### Client Library | ||
|
|
||
| #### NOTICE: Breaking API Changes in master branch |
There was a problem hiding this comment.
This isn't technically true yet, but I plan to merge open PRs soon that will make this true... so may as well prematurely optimize. 😄
theckman
force-pushed
the
better_last_response_handling
branch
2 times, most recently
from
b0805db
to
919619a
Compare
theckman
added a commit
that referenced
this pull request
Jul 4, 2021
This is to help with debugging issue #339. This branch is based off of #325, so that I could make use of the new response debugging functionality I added. I ran the following commands from within the `command` directory to get the below result: ```shell go build -o pd . ./pd schedule override create -authtoken "$(cat ../api.key)" PF4QFIN debug_override.json ``` The `api.key` file is just an API key from the PD console. `PF4QFIN` is a PagerDuty service in my development account. Here is the output: ``` time="2021-07-04T01:00:24-07:00" level=info msg="service id is:PF4QFIN" time="2021-07-04T01:00:24-07:00" level=info msg="Input file is:debug_override.json" req: req.Method: POST req.URL: https://api.pagerduty.com/schedules/PF4QFIN/overrides req.Header: http.Header{"Accept":[]string{"application/vnd.pagerduty+json;version=2"}, "Authorization":[]string{"Token token=u+XP5yPH7pzsQ3aJA5EQ"}, "Content-Type":[]string{"application/json"}, "User-Agent":[]string{"go-pagerduty/1.5.0"}} req.Body: {"override":{"start":"2021-06-29T12:00:00Z","end":"2021-06-29T18:00:00Z","user":{"id":"P6QKA9O"}}} err: HTTP response with status code 400, JSON error object decode failed: json: cannot unmarshal string into Go struct field APIError.error of type []string resp: resp.Status: 400 Bad Request resp.Header: http.Header{"Access-Control-Allow-Headers":[]string{"Authorization, Content-Type, AuthorizationOauth, X-EARLY-ACCESS"}, "Access-Control-Allow-Methods":[]string{"GET, POST, PUT, DELETE, OPTIONS"}, "Access-Control-Allow-Origin":[]string{"*"}, "Access-Control-Expose-Headers":[]string{""}, "Access-Control-Max-Age":[]string{"1728000"}, "Cache-Control":[]string{"no-cache"}, "Connection":[]string{"keep-alive"}, "Content-Length":[]string{"95"}, "Content-Type":[]string{"application/json"}, "Date":[]string{"Sun, 04 Jul 2021 08:00:24 GMT"}, "Server":[]string{"nginx"}, "X-Request-Id":[]string{"f6cda459-587c-49a6-ab98-17fc5bd60642"}} resp.Body: {"error":{"message":"Invalid Override","code":2001,"errors":"Invalid input for this override"}} ``` Note that `error.errors` is not an array, it's a single string. This seems to violate the API spec.
theckman
force-pushed
the
better_last_response_handling
branch
2 times, most recently
from
8796c89
to
26d0025
Compare
In prepration for #305, this adds a mechanism to inspect all API requests and responses handled by the client. That way if you want to figure out why something isn't working as expected, or you'd like to make use of an undocumented field/feature, you have access to the response and can use it. This also adds a `Do()` method, with the same signature as `*http.Client.Do()`, that allows consumrs to generate their own request, have the client add authentication details and other headers, before sending the request to the PagerDuty API. This allows consumers to further debug and make use of features not yet supported in the REST client. Also, updates Version string to v1.5.0 as this is work towards that release. Lastly, this updates some of the test files to remove some duplication across them. These changes were needed due to the new fields added to the struct that had to be initialized to make HTTP requests.
theckman
force-pushed
the
better_last_response_handling
branch
from
26d0025
to
4f01c5b
Compare
| authToken: "foo", | ||
| HTTPClient: defaultHTTPClient, | ||
| } | ||
| client := defaultTestClient(server.URL, "foo") |
There was a problem hiding this comment.
Technically, shouldn't these also be passing values for the v2EventsAPIEndpoint?
There was a problem hiding this comment.
stmcallister
approved these changes
Aug 24, 2021
This was referenced Sep 3, 2021
Closed
theckman
added a commit
that referenced
this pull request
Sep 4, 2021
This is a breaking change. This is one of the changes that will need to be made to complete issue #305, which is being done in a minor release even though it does contain breaking changes. The internal implementation details of how this was implemented meant it never worked in a way that consumers could use, because the body was always empty. Seeing as this API never worked and we do not wish to support it, we are going to remove it so that anyone who may have depended on it, but missed that it was broken, can be made aware. If someone wanted to capture the full response of these API calls, they can now do so using the the functionality added in #325 (4f01c5b).
theckman
added a commit
that referenced
this pull request
Sep 4, 2021
This is a breaking change. This is one of the changes that will need to be made to complete issue #305, which is being done in a minor release even though it does contain breaking changes. The internal implementation details of how this was implemented meant it never worked in a way that consumers could use, because the body was always empty. Seeing as this API never worked and we do not wish to support it, we are going to remove it so that anyone who may have depended on it, but missed that it was broken, can be made aware. If someone wanted to capture the full response of these API calls, they can now do so using the the functionality added in #325 (4f01c5b).
theckman
added a commit
that referenced
this pull request
Sep 4, 2021
This is a breaking change. This is one of the changes that will need to be made to complete issue #305, which is being done in a minor release even though it does contain breaking changes. The internal implementation details of how this was implemented meant it never worked in a way that consumers could use, because the body was always empty. Seeing as this API never worked and we do not wish to support it, we are going to remove it so that anyone who may have depended on it, but missed that it was broken, can be made aware. If someone wanted to capture the full response of these API calls, they can now do so using the the functionality added in #325 (4f01c5b).
theckman
added a commit
that referenced
this pull request
Sep 4, 2021
This is a breaking change. This is one of the changes that will need to be made to complete issue #305, which is being done in a minor release even though it does contain breaking changes. The internal implementation details of how this was implemented meant it never worked in a way that consumers could use, because the body was always empty. Seeing as this API never worked and we do not wish to support it, we are going to remove it so that anyone who may have depended on it, but missed that it was broken, can be made aware. If someone wanted to capture the full response of these API calls, they can now do so using the the functionality added in #325 (4f01c5b).
theckman
added a commit
that referenced
this pull request
Sep 4, 2021
This is a breaking change. This is one of the changes that will need to be made to complete issue #305, which is being done in a minor release even though it does contain breaking changes. The internal implementation details of how this was implemented meant it never worked in a way that consumers could use, because the body was always empty. Seeing as this API never worked and we do not wish to support it, we are going to remove it so that anyone who may have depended on it, but missed that it was broken, can be made aware. If someone wanted to capture the full response of these API calls, they can now do so using the the functionality added in #325 (4f01c5b).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In prepration for #305, this adds a mechanism to inspect all API requests and
responses handled by the client. That way if you want to figure out why
something isn't working as expected, or you'd like to make use of an
undocumented field/feature, you have access to the response and can use it.
This also adds a
Do()method, with the same signature as*http.Client.Do(),that allows consumrs to generate their own request, have the client add
authentication details and other headers, before sending the request to the
PagerDuty API. This allows consumers to further debug and make use of features
not yet supported in the REST client.
Also, updates Version string to v1.5.0 as this is work towards that release.
Lastly, this updates some of the test files to remove some duplication across
them. These changes were needed due to the new fields added to the struct that
had to be initialized to make HTTP requests.