microsoftgraph / msgraph-sdk-go-core

Microsoft Graph SDK for Go - Core Library
https://learn.microsoft.com/graph/sdks/sdks-overview
MIT License
20 stars 10 forks source link

method.IsNil() panics and the actual error is never returned #245

Closed manisharma closed 1 year ago

manisharma commented 1 year ago

Overview

Generic func convertToPage[T interface{}](response interface{}) (PageResult[T], error) method uses IsNil() method from reflect package eg: method.IsNil() to check if the response argument is Nil and as a result it panics and the actual/intended error is never returned. The PR addresses the panic, by replacing method.IsNil() check with method.Kind() == reflect.Invalid

manisharma commented 1 year ago

Thanks for the contribution. Would you mind adding a unit test to prevent future regressions please?

Done.

manisharma commented 1 year ago

@microsoft-github-policy-service agree

baywet commented 1 year ago

@manisharma your last changes are failing to test, would you mind taking another look before I review the changes please?

manisharma commented 1 year ago

pre-existing tests are failing @baywet

baywet commented 1 year ago

@manisharma it isn't on main if some additional unit tests are failing because of the change you've made, please update them.

manisharma commented 1 year ago

Yes thanks. I deleted pre-existing setup unknowingly. @baywet