Skip to content

Conversation

@Ndiritu
Copy link
Contributor

@Ndiritu Ndiritu commented Aug 26, 2021

Throws a GraphServiceException containing an error object for 4xx and 5xx responses

Closes #19

@Ndiritu Ndiritu force-pushed the feat/error-content branch from 41f01f8 to 48468a1 Compare August 31, 2021 07:28
@ddyett
Copy link

ddyett commented Sep 2, 2021

please open issues to link these PRs to, so that work can be tracked.

@Ndiritu
Copy link
Contributor Author

Ndiritu commented Sep 3, 2021

please open issues to link these PRs to, so that work can be tracked.

The issues are currently on the v1 repo. Working with the team to transfer all Core issues to this repo & switch to tracking high level goals using ZenHub

@Ndiritu Ndiritu marked this pull request as draft September 15, 2021 14:44
@Ndiritu Ndiritu marked this pull request as ready for review September 16, 2021 17:22
@Ndiritu
Copy link
Contributor Author

Ndiritu commented Sep 16, 2021

@MIchaelMainer updated this PR to throw native InvalidArgumentException for validation errors, GraphClientException for 4xx and GraphServiceException for 5xx

@Ndiritu
Copy link
Contributor Author

Ndiritu commented Sep 16, 2021

please open issues to link these PRs to, so that work can be tracked.

The issues are currently on the v1 repo. Working with the team to transfer all Core issues to this repo & switch to tracking high level goals using ZenHub

@ddyett @shemogumbe @roinochieng Core issues have been transferred to this repo and linked to epics to the best of my knowledge (subject to review & change with the rest of the team)

Copy link
Contributor

@MIchaelMainer MIchaelMainer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conditionally approved per the answer to my question: This change brings up another consideration. GraphClientException::GraphResponseException is very appropriate for 4xx errors, how do we specify a client exception that does not represent a 4xx error response?

We may want something like:
GraphClientException::Exception -- exceptions that occur before an HTTP request is made, or more generally, applicable to how the client throws an exception independent of request/response.
GraphClientResponseException::GraphResponseException -- 4xx exception.

I'd like to get your thoughts.

@Ndiritu
Copy link
Contributor Author

Ndiritu commented Sep 20, 2021

Conditionally approved per the answer to my question: This change brings up another consideration. GraphClientException::GraphResponseException is very appropriate for 4xx errors, how do we specify a client exception that does not represent a 4xx error response?

We may want something like:
GraphClientException::Exception -- exceptions that occur before an HTTP request is made, or more generally, applicable to how the client throws an exception independent of request/response.
GraphClientResponseException::GraphResponseException -- 4xx exception.

I'd like to get your thoughts.

Unable to figure out a use-case for a non-4xx client exception that wouldn't be covered by a native exception type e.g. InvalidArgumentException for validation errors etc.

My understanding from the Teams thread was that all other exceptions would be native?

@MIchaelMainer
Copy link
Contributor

That's fair.

@Ndiritu Ndiritu merged commit c9bb7fc into feat/page-iterator Sep 21, 2021
@Ndiritu Ndiritu deleted the feat/error-content branch October 28, 2021 18:48
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

Successfully merging this pull request may close these issues.

3 participants