Skip to content

Updated file to handle non json responses #1457

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

Merged
merged 4 commits into from
Aug 18, 2022

Conversation

timayabi2020
Copy link
Contributor

@timayabi2020 timayabi2020 commented Aug 13, 2022

Fixes #1425
This PR addresses an issue raised here

Changes proposed in this pull request

  • Invoke-MgGraphRequest throws an error requiring -OutputFilePath when it can't process non-JSON responses
  • Writes results to console if -OutputType HttpResponseMessage argument is provided

Throws an error

image

Writes result to console

image

Writes result to file

image

Comment on lines 602 to 604
var responseType = response.GetType();
var contentType = response.GetContentType();
if (responseType != typeof(HttpResponseMessage) || (contentType.Equals("text/plain") && String.IsNullOrEmpty(OutputFilePath) && !OutputType.Equals(OutputType.HttpResponseMessage)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-OutputFilePath is only needed for non-json responses. Let's change the contentType condition to !contentType.Equals("application/json").

@peombwa
Copy link
Member

peombwa commented Aug 16, 2022

@timayabi2020, we should return the HTTP response object as a HttpResponseMessage type when -OutputType HttpResponseMessage is specified. See existing switch statement for -OutputType at

case OutputType.HttpResponseMessage:
WriteObject(response);
break;

@peombwa
Copy link
Member

peombwa commented Aug 16, 2022

@timayabi2020, can't we fix #1425 by changing:

var returnType = response.CheckReturnType();
if (returnType == RestReturnType.Json)
{
var responseString = await response.Content.ReadAsStringAsync();
ErrorRecord error;
switch (OutputType)
{
case OutputType.HashTable:
var hashTable = responseString.ConvertFromJson(true, null, out error);
ThrowIfError(error);
WriteObject(hashTable);
break;
case OutputType.PSObject:
var psObject = responseString.ConvertFromJson(false, null, out error);
ThrowIfError(error);
WriteObject(psObject, true);
break;
case OutputType.HttpResponseMessage:
WriteObject(response);
break;
case OutputType.Json:
WriteObject(responseString);
break;
default:
throw new ArgumentOutOfRangeException();
}
}
else if (returnType == RestReturnType.Image)
{
var errorRecord =
GetValidationError(Resources.NonJsonResponseWithoutOutputFilePath,
ErrorConstants.Codes.InvokeGraphContentTypeException, returnType);
ThrowIfError(errorRecord);
}
else if (returnType == RestReturnType.Detect)
{
var responseString = await response.Content.ReadAsStringAsync();
WriteObject(responseString);
}
else if (returnType == RestReturnType.OctetStream)
{
var errorRecord =
GetValidationError(Resources.NonJsonResponseWithoutInfer,
ErrorConstants.Codes.InvokeGraphContentTypeException, returnType, response.Content.Headers.ContentDisposition);
ThrowIfError(errorRecord);
}
to:

var returnType = response.CheckReturnType();
switch (returnType)
{
    case RestReturnType.Json:
        ErrorRecord error;
        string responseString;
        switch (OutputType)
        {
            case OutputType.HashTable:
                responseString = await response.Content.ReadAsStringAsync();
                var hashTable = responseString.ConvertFromJson(true, null, out error);
                ThrowIfError(error);
                WriteObject(hashTable);
                break;
            case OutputType.PSObject:
                responseString = await response.Content.ReadAsStringAsync();
                var psObject = responseString.ConvertFromJson(false, null, out error);
                ThrowIfError(error);
                WriteObject(psObject, true);
                break;
            case OutputType.HttpResponseMessage:
                WriteObject(response);
                break;
            case OutputType.Json:
                responseString = await response.Content.ReadAsStringAsync();
                WriteObject(responseString);
                break;
            default:
                throw new ArgumentOutOfRangeException(nameof(OutputType));
        }
        break;
    case RestReturnType.OctetStream:
        if (OutputType == OutputType.HttpResponseMessage)
            WriteObject(response);
        else
            ThrowIfError(GetValidationError(Resources.NonJsonResponseWithoutInfer, ErrorConstants.Codes.InvokeGraphContentTypeException, returnType, response.Content.Headers.ContentDisposition));
        break;
    default:
        if (OutputType == OutputType.HttpResponseMessage)
            WriteObject(response);
        else
            ThrowIfError(GetValidationError(Resources.NonJsonResponseWithoutOutputFilePath, ErrorConstants.Codes.InvokeGraphContentTypeException, returnType));
        break;
}

This approach limits the change to just ProcessResponseAsync method.

@timayabi2020 timayabi2020 merged commit 5150945 into dev Aug 18, 2022
@peombwa peombwa mentioned this pull request Aug 23, 2022
@peombwa peombwa deleted the InvokeMgGraphRequest_Handle_NonJsonResponse branch October 14, 2022 16:16
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

Successfully merging this pull request may close these issues.

Invoke-MgGraphRequest should throw an error when it can't process non-JSON responses
2 participants