Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic when parsing Photo facet #685

Closed
bartek opened this issue Apr 5, 2024 · 11 comments
Closed

Panic when parsing Photo facet #685

bartek opened this issue Apr 5, 2024 · 11 comments
Labels
blocked resolving this issue is blocked by an upstream dependency bug Something isn't working service issue Issues caused by the service (metadata/behavior)

Comments

@bartek
Copy link

bartek commented Apr 5, 2024

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
@gil-laminar
Copy link

+1, we are also encountering this issue

@gil-laminar
Copy link

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.

@bartek
Copy link
Author

bartek commented Apr 9, 2024

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.

@gil-laminar
Copy link

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

@baywet baywet added bug Something isn't working service issue Issues caused by the service (metadata/behavior) blocked resolving this issue is blocked by an upstream dependency labels Apr 16, 2024
@baywet
Copy link
Member

baywet commented Apr 16, 2024

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!

@abezzub
Copy link

abezzub commented May 21, 2024

This brought down our integration, any ETA on resolution?

@coderavels
Copy link

coderavels commented May 21, 2024

@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?

@bartek
Copy link
Author

bartek commented May 21, 2024

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
}

@baywet
Copy link
Member

baywet commented May 21, 2024

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

@baywet
Copy link
Member

baywet commented Jul 31, 2024

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 baywet closed this as completed Jul 31, 2024
@kyong0612
Copy link

kyong0612 commented Aug 30, 2024

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked resolving this issue is blocked by an upstream dependency bug Something isn't working service issue Issues caused by the service (metadata/behavior)
Projects
None yet
Development

No branches or pull requests

6 participants