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

Upload app logo resulted in Error #745

Open
ritu-rubrik opened this issue Jul 13, 2024 · 7 comments
Open

Upload app logo resulted in Error #745

ritu-rubrik opened this issue Jul 13, 2024 · 7 comments
Assignees

Comments

@ritu-rubrik
Copy link

Using the below code to upload an Logo for a given App:

headers := abstractions.NewRequestHeaders()
	headers.Add("Content-Type", "image/jpg")
	configuration := &applications.
		ItemLogoRequestBuilderPutRequestConfiguration{
		Headers: headers,
	}
	appLogo, err := os.ReadFile(appLogoFile)
	resp, err := client.Applications().ByApplicationId(appObjectID).Logo().Put(
		ctx,
		appLogo,
		configuration,
	)

However this results in the below error:
content type text/html does not have a factory registered to be parsed
On debugging further it was found that the APi call results in a a Bad Request 400 erro and the same is getting set in response body resulting in above error.
Looks like Content-tye is set to 2 values image/jpeg and application/octet-stream . Not sure if this is causeing the request to the MS graph API to fail

@ritu-rubrik ritu-rubrik added the status:waiting-for-triage An issue that is yet to be reviewed or assigned label Jul 13, 2024
@rkodev rkodev self-assigned this Jul 15, 2024
@rkodev
Copy link
Contributor

rkodev commented Jul 15, 2024

Hi @ritu-rubrik , Thanks for trying the Graph Go SDK. Could you retry the code without setting the content header
i.e remove the line explicitly setting content header headers.Add("Content-Type", "image/jpg")

headers := abstractions.NewRequestHeaders()
-	headers.Add("Content-Type", "image/jpg")
	configuration := &applications.
		ItemLogoRequestBuilderPutRequestConfiguration{
		Headers: headers,
	}

@rkodev rkodev added the status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close label Jul 15, 2024
@ritu-rubrik
Copy link
Author

ritu-rubrik commented Jul 15, 2024

Hi @rkodev I had initially tried it without setting the Content-Type

headers := abstractions.NewRequestHeaders()
	configuration := &applications.
		ItemLogoRequestBuilderPutRequestConfiguration{
		Headers: headers,
	}
	appLogo, err := os.ReadFile(appLogoFile)

	resp, err := client.Applications().ByApplicationId(appObjectID).Logo().Put(
		ctx,
		appLogo,
		configuration,
	)

However I ended up getting the below error which resulted in me adding the Content-Type:
Content-Type was application/octet-stream. Image Content-Type header should be one of the following: [image/bmp, image/emf, image/wmf, image/jpg, image/jpeg, image/gif, image/ico, image/png, image/exif, image/tif, image/tiff, image/*, images/*].

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs Attention 👋 and removed status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close labels Jul 15, 2024
@ritu-rubrik
Copy link
Author

Also one another question I have is if the response is Bad Request , why is it not getting set in OData Error as it is a 4XX status code, instead it is getting set in the response body which is leading to the initial error.content type text/html does not have a factory registered to be parsed

@rkodev
Copy link
Contributor

rkodev commented Jul 15, 2024

Thanks for pointing this out,

  1. I will be working on a solution on allowing users to override Content-Type which would be useful to in this case. As you correctly pointed out, whatever Content-Type header have right now, the sdk will override and set it to octet-stream. We will not be able to set the content type on the sdk at this moment as this SDK is "generated".

  2. Also I've noted the issue with the issue with error management. Issue seems to be the underlying service accepts a payload with content type 'application/json' but reports the error as 'text/html'. We will be working on that as well

@rkodev rkodev removed Needs Attention 👋 status:waiting-for-triage An issue that is yet to be reviewed or assigned labels Jul 15, 2024
@ritu-rubrik ritu-rubrik changed the title Upload app logo resulet in Error Upload app logo resulted in Error Jul 16, 2024
@ritu-rubrik
Copy link
Author

@rkodev Thanks for acknowledging the issue. Is there a timeline for this that I can expect for this change to be available?

@rkodev
Copy link
Contributor

rkodev commented Jul 17, 2024

@ritu-rubrik The fix should be ready within the next 7 days

@baywet
Copy link
Member

baywet commented Jul 17, 2024

@rkodev Can you open a corresponding issue in the metadata repo please?
Fundamentally, the API description should not describe a media type for the request body if the service does not support it.
Addressing the issue in the metadata emission phase would fix the issue for all generated clients, not just go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants