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 while uploading certain item attachments #667

Open
HiteshRepo opened this issue Feb 23, 2024 · 15 comments
Open

Panic while uploading certain item attachments #667

HiteshRepo opened this issue Feb 23, 2024 · 15 comments
Labels
question Further information is requested service issue Issues caused by the service (metadata/behavior)

Comments

@HiteshRepo
Copy link

HiteshRepo commented Feb 23, 2024

We encounter panic: were unable to deserialize while trying to upload attchements using below API:

client.
Users().
ByUserId(userID).
MailFolders().
ByMailFolderId(containerID).
Messages().
ByMessageId(parentItemID).
Attachments().
Post(ctx, body, nil)

client is of type *msgraphsdkgo.GraphServiceClient.

Below are couple of sample item attachement jsons for which we get the above panics:

However we do not get the issue for below:
non-nested item attachment #2

Please advise us in case there is something wrong with the structure/content of the item attachement jsons that is leading to the panic.
Or is there is any issue with any of the below libs:

In either case we should not be getting panics rather some appropriate error message.

@baywet baywet added question Further information is requested Needs Attention 👋 labels Feb 23, 2024
@rkodev rkodev self-assigned this Feb 28, 2024
@saviorand
Copy link

@andrueastman @rkodev @baywet any update on this? We are getting a panic where e.g a "dimensions" property on a File in a Drive of type Photo is a string, and not a float.

Based on this discussion in kiota-serialization-go microsoft/kiota-serialization-json-go#142 , this is intended behaviour by kiota but not handled properly by msgraph-sdk-go. This then causes a panic that prevents the user from retrieving any files of the graph SDK.

Should I create a separate issue for this?

@michaeldcanady
Copy link

@saviorand Would you be able to provide the JSON you’re passing in and how are you generating the JSON?

@michaeldcanady
Copy link

Apologies I see your examples. Can you verify your version of the JSON serializer is the latest and where you able to pinpoint what key is causing the issue? After a cursory glance I don’t see any that appear like the expected error case I outlined.

@andrueastman
Copy link
Member

@saviorand As suggested by @michaeldcanady, Any chance you can confirm this is happening if you have the latest version of the sdk?

@andrueastman andrueastman added status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close and removed Needs Attention 👋 labels Sep 9, 2024
@saviorand
Copy link

@michaeldcanady @andrueastman I am on [email protected];

just tried running it again, I'm getting errors:CLIENT_FAILURE: failed to get drive items: *errors.errorString > error: &errors.errorString{s:"value '1' is not compatible with type int32"}

when I run this in the debugger, the error is thrown here: /go/pkg/mod/github.com/microsoftgraph/[email protected]/models/photo.go. I marked with a comment the exact spot where the error is thrown:

res["orientation"] = func (n (omitted).ParseNode) error {  
    val, err := n.GetInt32Value()  
    if err != nil {  // value '1' is not compatible with type int32
        return err  
    }  
    if val != nil {  
        m.SetOrientation(val)  
    }  
    return nil  
}

Is this not the latest version? I think I just did go get https://github.com/microsoftgraph/msgraph-sdk-go@main

@saviorand
Copy link

Here is the file (photo) I'm using if it's helpful, but I'm sure the issue can be reproduced with any file that has a string representation of an integer in the "orientation" field

@michaeldcanady
Copy link

michaeldcanady commented Sep 9, 2024

@saviorand what version of the JSON serializer are you using?
My fix wasn’t released until 1.0.8.

@saviorand
Copy link

@michaeldcanady I am just calling msgraph-sdk-go , kiota-serialization is an indirect dependency for me. Looks like it's at 1.0.8 in my go.mod

@baywet
Copy link
Member

baywet commented Sep 9, 2024

@andrueastman this is related to #685, the workload (OSDP) updated the service to return ints.
We should open an ICM on this workload (Exchange) so they also fix their API to return ints.

@saviorand
Copy link

@baywet am I right to assume this might take a while for that team to get back with a fix? Based on the discussion in the other issue.

@baywet
Copy link
Member

baywet commented Sep 16, 2024

@saviorand yes, even when the teams are reactive and prioritize the work, the deployment takes weeks.

@andrueastman
Copy link
Member

@saviorand Any chance you can confirm the api you are calling in this scenario?
The orientation property seems to be from photo model.

res["orientation"] = func (n i878a80d2330e89d26896388a3f487eef27b0a0e6c010c493bf80be1452208f91.ParseNode) error {

However the original issue was calling the attachments api as below.

client.
Users().
ByUserId(userID).
MailFolders().
ByMailFolderId(containerID).
Messages().
ByMessageId(parentItemID).
Attachments().
Post(ctx, body, nil)

We may need to follow up with the ODSP team again about this depending on which API is being called.

@saviorand
Copy link

saviorand commented Sep 17, 2024

@andrueastman @baywet yup it's in the Photo model, and this is the call I'm making where it panics, so the Drive API
driveItemsResult, err := client.Drives().ByDriveId(driveID).Items().ByDriveItemId(itemToSearch).Children().Get( ctx, nil )

@andrueastman
Copy link
Member

Thanks for the info @saviorand

Looks to be the exact scenario as #685.

Following up with the relevant team at https://portal.microsofticm.com/imp/v5/incidents/details/544783286/summary and will give feedback once we get confirmation on the current state of things here.

@andrueastman andrueastman added service issue Issues caused by the service (metadata/behavior) and removed status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close labels Sep 18, 2024
@saviorand
Copy link

@andrueastman thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested service issue Issues caused by the service (metadata/behavior)
Projects
None yet
Development

No branches or pull requests

6 participants