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

Teams AddAsync/PostAsync is totally and utterly broken

Open davhdavh opened this issue 2 years ago • 7 comments

Describe the bug A reasonable person might think that this was the correct usage of the API: var newteam = await GraphClient.Teams.Request().AddAsync(team, cancellationToken: CancellationToken); but noooo, that just returns null.

Another reasonable person might see that there is a AddResponseAsync and try:

var channelResponse = await GraphClient.Teams.Request().AddResponseAsync(team, cancellationToken: CancellationToken);
var newteam = await channelResponse.GetResponseObjectAsync();

bot noooooo, that also just returns null. (FYI, there is no overload of GetResponseObjectAsync that takes CancellationToken, which is a bug in itself)

A totally fucking idiot might design the API to actually be called in this way:

         var v = await PostInTaskChannel(MessageFactory.Text(JsonConvert.SerializeObject(team)));
         var channelResponse = await GraphClient.Teams.Request().AddResponseAsync(team, cancellationToken: CancellationToken);
         if (!channelResponse.HttpHeaders.TryGetValues("Location", out var headerValues) ||
             headerValues.First().Split('\'', StringSplitOptions.RemoveEmptyEntries) is not { Length: > 3 } operationRequest)
            throw new("Unknown error, the request did not return the proper response:" + JsonConvert.SerializeObject(channelResponse.HttpHeaders));
         TeamsAsyncOperation? operation = null;
         var firstRequest = true;
         do
         {
            try
            {
               operation = await GraphClient.Teams[operationRequest[1]].Operations[operationRequest[3]].Request().GetAsync(CancellationToken);
               firstRequest = false;
            }
            catch
            {
               if (firstRequest) //sometimes it cannot find the operation, presumable because of hitting another server
                  operation = new() { Status = TeamsAsyncOperationStatus.InProgress };
               else throw;
            }
            if (operation.Status == TeamsAsyncOperationStatus.InProgress)
               await Task.Delay(TimeSpan.FromMilliseconds(100));
         } while (operation.Status is TeamsAsyncOperationStatus.InProgress);

         if (operation.Status is not (TeamsAsyncOperationStatus.Succeeded or TeamsAsyncOperationStatus.NotStarted))
            throw new(JsonConvert.SerializeObject(operation.Error));

         var newTeam = await GraphClient.Teams[operationRequest[1]].Request().GetAsync(CancellationToken);

And why is the last outcome of an successful operation TeamsAsyncOperationStatus.NotStarted?!?!?!?!?

Expected behavior var newteam = await GraphClient.Teams.Request().AddAsync(team, cancellationToken: CancellationToken); works... or atleast returns an "TeamsCreationAsyncResult" that can be used to query the further result.

Client version 4.29

davhdavh avatar Jun 02 '22 05:06 davhdavh

Just wait until your call to Teams[].Operations[] returns a 404. You are left with nothing...

The workaround is to query Teams using the displayName and hope that your new team name didn't get adjusted because of duplicates...

pschaeflein avatar Jun 02 '22 13:06 pschaeflein

No updates on this issue?

elarrieux-imanage avatar Jul 13 '22 19:07 elarrieux-imanage

Hi @andrueastman . Can you please take a look at this?

maisarissi avatar Jul 15 '22 20:07 maisarissi

The following call results to the equivalent of the documented api call here. As the API returns no response body the returned value is null.

var newteam = await GraphClient.Teams.Request().AddAsync(team, cancellationToken: CancellationToken);

Due to the metadata used for SDK generation, the correct return type for this and other related functions should be resolved as we move to accurate description using OpenAPI in the next major version of the SDK.

The Core library also provides the AsyncMonitor class that should be ideally used for the checking of async operations on the Graph API and provide a developer experience close to this.

// create the team
var team = new Team
{
    DisplayName = "My Sample Team",
    Description = "My Sample Team’s Description",
    AdditionalData = new Dictionary<string, object>()
    {
        {"[email protected]", "https://graph.microsoft.com/v1.0/teamsTemplates('standard')"}
    }
};

var teamsCreateReponse = await graphClient.Teams
    .Request()
    .AddResponseAsync(team);

var locationHeader = teamsCreateReponse.HttpHeaders.Location;

if (locationHeader != null)
{
    if (!locationHeader.IsAbsoluteUri)
        locationHeader =  new Uri(graphClient.BaseUrl + locationHeader.OriginalString);
    
    // create the monitor
    var asyncMonitor = new AsyncMonitor<Team>(graphClient, locationHeader.AbsoluteUri);
    IProgress<AsyncOperationStatus> progress = new Progress<AsyncOperationStatus>(operationStatus => 
    {
        Console.WriteLine($"Operation status is {operationStatus.Status} complete");
    });

    var createdTeam = await asyncMonitor.PollForOperationCompletionAsync(progress, CancellationToken.None);
    Console.WriteLine($"Team created with id: {createdTeam.Id}");
}

However, on testing, it seems the teams API returns 200 response instead of the expected 202 response in the AsyncMonitor at the link below to result in the monitor exiting early for the teams scenario.

https://github.com/microsoftgraph/msgraph-sdk-dotnet-core/blob/50ab54e8f17421fc1564f1fc5600b35197a245db/src/Microsoft.Graph.Core/Requests/AsyncMonitor.cs#L51

To resolve this, we would need to update the monitor code to also allow for scenarios where the monitor endpoint returns 200 response status code as well.

andrueastman avatar Aug 05 '22 09:08 andrueastman

A new timing bug seems to have entered the Teams Bot world. Before going on vacation, creating a new team using the Graph API with the Bot included in the InstalledApps and then sending a message using SendMessageToTeamsChannelAsync immediately upon the successful return of the Operation would result in a successful first message in the channel... Now, however, it returns with a FORBIDDEN and the channel log in Azure says "The bot is not part of the conversation roster."

davhdavh avatar Aug 18 '22 09:08 davhdavh

With regards to the AsyncMonitor, then it is not really compatible with Teams at all

  • AsyncOperationStatus.Status != TeamsAsyncOperation.Status
  • AsyncOperationStatus.AdditionalData.TryGetValue("message", out message) != TeamsAsyncOperation.Error
  • The 200 vs 202 as you mention

davhdavh avatar Aug 18 '22 09:08 davhdavh

https://github.com/microsoftgraph/msgraph-sdk-design/issues/83

andrueastman avatar Jan 27 '23 12:01 andrueastman