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

Panic when parsing Photo facet

Open bartek opened this issue 10 months ago • 9 comments

Hello!

Ran into an issue during indexing of drive items where, when the photo facet is present, the Go SDK panics. Here's an example facet from the MS Graph API (v1.0):

{
  "photo": {
    "cameraMake": "Apple",
    "cameraModel": "iPhone 15",
    "exposureDenominator": 742,
    "exposureNumerator": 1,
    "fNumber": 1.6,
    "focalLength": 5.96,
    "iso": 50,
    "orientation": "6",
    "takenDateTime": "2024-04-02T17:21:10.37Z"
  }
}

The panic with (relevant) stack trace is as follows:

panic: interface conversion: interface {} is *string, not *float64

goroutine 118 [running]:
runtime/debug.Stack()
	/usr/local/go/src/runtime/debug/stack.go:24 +0x64
github.com/atolio/lumen/pkg/sdk/runner.(*Runner).executeWithRecover.func1()
	/go/src/pkg/sdk/runner/runner.go:371 +0x114
panic({0x31c9740?, 0x4001814cc0?})
	/usr/local/go/src/runtime/panic.go:914 +0x218
github.com/microsoft/kiota-serialization-json-go.(*JsonParseNode).GetFloat64Value(...)
	/go/pkg/mod/github.com/microsoft/[email protected]/json_parse_node.go:406
github.com/microsoft/kiota-serialization-json-go.(*JsonParseNode).GetInt32Value(0x400144ce40?)
	/go/pkg/mod/github.com/microsoft/[email protected]/json_parse_node.go:411 +0xa0
github.com/microsoftgraph/msgraph-sdk-go/models.(*Photo).GetFieldDeserializers.func9({0x4471050?, 0x400144ce40?})
	/go/pkg/mod/github.com/microsoftgraph/[email protected]/models/photo.go:170 +0x34
github.com/microsoft/kiota-serialization-json-go.(*JsonParseNode).GetObjectValue(0x400144ce80, 0x400143de40?)
	/go/pkg/mod/github.com/microsoft/[email protected]/json_parse_node.go:225 +0x368
github.com/microsoftgraph/msgraph-sdk-go/models.(*DriveItem).GetFieldDeserializers.func18({0x4471050?, 0x400144ce80?})
	/go/pkg/mod/github.com/microsoftgraph/[email protected]/models/drive_item.go:287 +0x3c
github.com/microsoft/kiota-serialization-json-go.(*JsonParseNode).GetObjectValue(0x400144cf00, 0x0?)
	/go/pkg/mod/github.com/microsoft/[email protected]/json_parse_node.go:225 +0x368
github.com/microsoft/kiota-serialization-json-go.(*JsonParseNode).GetCollectionOfObjectValues(0x4001517e80?, 0x41bfd78)
	/go/pkg/mod/github.com/microsoft/[email protected]/json_parse_node.go:251 +0x16c
github.com/microsoftgraph/msgraph-sdk-go/drives.(*ItemItemsItemDeltaResponse).GetFieldDeserializers.(*ItemItemsItemDeltaGetResponse).GetFieldDeserializers.func1({0x4471050?, 0x4001517e40?})
	/go/pkg/mod/github.com/microsoftgraph/[email protected]/drives/item_items_item_delta_get_response.go:27 +0x3c
github.com/microsoft/kiota-serialization-json-go.(*JsonParseNode).GetObjectValue(0x4001517e60, 0x44627e0?)
	/go/pkg/mod/github.com/microsoft/[email protected]/json_parse_node.go:225 +0x368
github.com/microsoft/kiota-http-go.(*NetHttpRequestAdapter).Send(0x4000523360, {0x4462818?, 0x400035d4a0?}, 0x40010803c0, 0x40010a4060?, 0x4000fe11e0?)
	/go/pkg/mod/github.com/microsoft/[email protected]/nethttp_request_adapter.go:359 +0x290
github.com/microsoftgraph/msgraph-sdk-go/drives.(*ItemItemsItemDeltaRequestBuilder).Get(0x4000a10f58, {0x4462818, 0x400035d4a0}, 0xf?)
	/go/pkg/mod/github.com/microsoftgraph/[email protected]/drives/item_items_item_delta_request_builder.go:63 +0xec

As per generated Photo, it seems that an int is expected but the MS Graph API is returning a string, resulting in this panic:

// models/photo.go
res["orientation"] = func (n i878a80d2330e89d26896388a3f487eef27b0a0e6c010c493bf80be1452208f91.ParseNode) error {
    val, err := n.GetInt32Value()
    if err != nil {
        return err
    }
    if val != nil {
        m.SetOrientation(val)
    }
    return nil
}   

In browsing other APIs which deal with orientation[1], it seems an int to represent orientation is more common.

Questions:

  1. Can the msgraph-sdk-go project be re-generated to solve the discrepancy between API/client? I am happy to help with this, but I could not find where the relevant spec(s) are managed.
  2. Should this be resolved in the API itself? I understand this may result in breaking issues for existing clients (which may be working around this issue). Curious what the typical flow is here

Thank you!

  1. https://developer.apple.com/documentation/uikit/uiimage/orientation

bartek avatar Apr 05 '24 15:04 bartek

+1, we are also encountering this issue

gil-laminar avatar Apr 09 '24 13:04 gil-laminar

Looking through some of the other languages' SDKs, it looks like they all agree on orientation being an Int32, while the API documentation says it's an Int16, and the server sends a string. My guess is that the server's wrong.

gil-laminar avatar Apr 09 '24 13:04 gil-laminar

FWIW, I worked around this by using $select to ensure I don't receive the photos facet. This is satisfactory for my use case as I don't need this facet for what I am doing, but of course not ideal in general.

bartek avatar Apr 09 '24 14:04 bartek

Thanks, this is what we're planning to do as well.

gil-laminar avatar Apr 09 '24 14:04 gil-laminar

Hi everyone, Thank you for using the Go SDK and for reporting this. You are correct the metadata (used to generate the SDK) and the documentation are not aligned with what the service sends. I've opened an incident on the relevant team (you won't be able to open that) https://portal.microsofticm.com/imp/v3/incidents/incident/494219554/summary

If some of you could share client request ids and timestamps for the requests, that'd be helpful as well. (even from Graph Explorer) Thanks!

baywet avatar Apr 16 '24 14:04 baywet

This brought down our integration, any ETA on resolution?

abezzub avatar May 21 '24 03:05 abezzub

@bartek I tried using select query and even then facing the issue. Here is the code we are using:

var (
	fieldsForGetDelta = []string{"id", "name", "webUrl", "lastModifiedDateTime", "createdDateTime", "createdBy", "lastModifiedBy", "file", "size", "parentReference", "deleted"}
)
...
...
...
var deltaResp drives.ItemItemsItemDeltaGetResponseable
	if url == "" {
		deltaResp, err = client.Drives().ByDriveId(driveID).Items().ByDriveItemId(itemID).Delta().GetAsDeltaGetResponse(ctx, &drives.ItemItemsItemDeltaRequestBuilderGetRequestConfiguration{
			QueryParameters: &drives.ItemItemsItemDeltaRequestBuilderGetQueryParameters{
				Select: fieldsForGetDelta,
			},
		})
		if err != nil {
			errResp := common.FormatODataError(err)
			return resp, core.TraceError(ctx, "error while getting onedrive drive item first delta", errResp)
		}
	} else {
		deltaResp, err = client.Drives().ByDriveId(driveID).Items().ByDriveItemId(itemID).Delta().WithUrl(url).GetAsDeltaGetResponse(ctx, &drives.ItemItemsItemDeltaRequestBuilderGetRequestConfiguration{
			QueryParameters: &drives.ItemItemsItemDeltaRequestBuilderGetQueryParameters{
				Select: fieldsForGetDelta,
			},
		})
		if err != nil {
			errResp := common.FormatODataError(err)
			return resp, core.TraceError(ctx, "error while getting onedrive drive item delta with url", errResp)
		}
	}

Here is the stack trace

github.com/watchtowerai/m365dlp/internal/m365/onedrive/client.azureGraphServiceImpl.GetDriveItemDelta.func2
	/projects/m365dlp/internal/m365/onedrive/client/azure_graph.go:242
runtime.gopanic
	/usr/local/go/src/runtime/panic.go:770
runtime.panicdottypeE
	/usr/local/go/src/runtime/iface.go:262
github.com/microsoft/kiota-serialization-json-go.(*JsonParseNode).GetFloat64Value
	/go/pkg/mod/github.com/microsoft/[email protected]/json_parse_node.go:463
github.com/microsoft/kiota-serialization-json-go.(*JsonParseNode).GetInt32Value
	/go/pkg/mod/github.com/microsoft/[email protected]/json_parse_node.go:468
github.com/microsoftgraph/msgraph-sdk-go/models.(*Photo).GetFieldDeserializers.func9
	/go/pkg/mod/github.com/watchtowerai/[email protected]/models/photo.go:170
github.com/microsoft/kiota-serialization-json-go.(*JsonParseNode).GetObjectValue
	/go/pkg/mod/github.com/microsoft/[email protected]/json_parse_node.go:282
github.com/microsoftgraph/msgraph-sdk-go/models.(*DriveItem).GetFieldDeserializers.func18
	/go/pkg/mod/github.com/watchtowerai/[email protected]/models/drive_item.go:287
github.com/microsoft/kiota-serialization-json-go.(*JsonParseNode).GetObjectValue
	/go/pkg/mod/github.com/microsoft/[email protected]/json_parse_node.go:282
github.com/microsoft/kiota-serialization-json-go.(*JsonParseNode).GetCollectionOfObjectValues
	/go/pkg/mod/github.com/microsoft/[email protected]/json_parse_node.go:308
github.com/microsoftgraph/msgraph-sdk-go/drives.(*ItemItemsItemDeltaGetResponse).GetFieldDeserializers.func1
	/go/pkg/mod/github.com/watchtowerai/[email protected]/drives/item_items_item_delta_get_response.go:27
github.com/microsoft/kiota-serialization-json-go.(*JsonParseNode).GetObjectValue
	/go/pkg/mod/github.com/microsoft/[email protected]/json_parse_node.go:282
github.com/microsoft/kiota-http-go.(*NetHttpRequestAdapter).Send
	/go/pkg/mod/github.com/microsoft/[email protected]/nethttp_request_adapter.go:359
github.com/microsoftgraph/msgraph-sdk-go/drives.(*ItemItemsItemDeltaRequestBuilder).GetAsDeltaGetResponse
	/go/pkg/mod/github.com/watchtowerai/[email protected]/drives/item_items_item_delta_request_builder.go:82

Could you help with what different you did to use the select query and prevent this issue? Also which version of graph-sdk are you using?

coderavels avatar May 21 '24 05:05 coderavels

Hey @coderavels. Not sure, I'd recommend logging the response to understand what may be getting parsed. We are accessing the data from the root, like so:

root, err := c.graphClient.Drives().ByDriveId(driveId).Root().Get(ctx, nil)
if err != nil {
    return nil, err
}

if root.GetId() == nil {
    return nil, fmt.Errorf("drive root ID is nil")
}
rootId := *root.GetId()    

deltaReqBldr := c.graphClient.Drives().ByDriveId(driveId).Items().ByDriveItemId(rootId).Delta()
if deltaLink != "" {
    deltaReqBldr = deltaReqBldr.WithUrl(deltaLink)
}

result, err := deltaReqBldr.GetAsDeltaGetResponse(ctx, &drives.ItemItemsItemDeltaRequestBuilderGetRequestConfiguration{
QueryParameters: &drives.ItemItemsItemDeltaRequestBuilderGetQueryParameters{
	Select: []string{
		"id", "lastModifiedDateTime", "name", "webUrl", "size",
		"createdBy", "lastModifiedBy", "parentReference", "file",
		"fileSystemInfo",
	},
},
})
if err != nil {
    return nil, err
}

bartek avatar May 21 '24 16:05 bartek

Thanks everyone for the nudge, I've nudged the concerned service team to get an update.

baywet avatar May 21 '24 16:05 baywet

Hi everyone, Thank you for your patience. While the service team never came back to me on the topic, I took the time to retry the call today and the API now returns integers. Closing.

baywet avatar Jul 31 '24 16:07 baywet

@bartek Did this bug is fix?

While the service team never came back to me on the topic, I took the time to retry the call today and the API now returns integers.

If the response is an integer, does a similar panic occur during type assertion to a float64?

kyong0612 avatar Aug 30 '24 04:08 kyong0612