msgraph-sdk-php icon indicating copy to clipboard operation
msgraph-sdk-php copied to clipboard

Service/SDK Design: Handling 404s when requested resource is null?

Open Ndiritu opened this issue 4 years ago • 2 comments

I actually started a new project quite recently and am in a position to switch to RC. Will do so happily.

Must say I'm surprised there's still no really good handling for null results from what I see. Specifically, querying /users/{id}/manager will return a 404 (correctly) for the CEO in any organization. Is there any reasonable way to handle this apart from catching the GraphClientException and inspecting the return code? Even though this implies using exceptions for regular expected program flow, which is of course bad practice by any standard?

Originally posted by @curry684 in https://github.com/microsoftgraph/msgraph-sdk-php/issues/681#issuecomment-957221678

Ndiritu avatar Nov 02 '21 10:11 Ndiritu

Same for photos btw (just trying):

Error thrown while running command "graph:azure:sync -svv". Message: "'GET' request to https://graph.microsoft.com/v1.0/users/{id}/photo/$value returned 404 {"error":{"code":"ImageNotFound","message":"Exception of type 'Microsoft.Fast.Profile.Core.Exception.ImageNotFoundException' was thrown."

Catching the exception should not be intended program flow.

curry684 avatar Nov 02 '21 11:11 curry684

@curry684

Instead of trying to access the manager resource directly with /users/{id}/manager, I suggest that you use expansion: /users/{id}?$expand=manager. If the user has a manager, the manager property will be populated on the user. If the user does not have a manager, the property will not be populated. This should also save you an extra call. I suggest you use this pattern when available.

Unfortunately, the photos endpoints doesn't work the same way. There is no way to query whether the user has a photo (or even the photo metadata), without trying to get the photo resource, and that means handling the resource not found error.

MIchaelMainer avatar Nov 02 '21 16:11 MIchaelMainer