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

Teams AddAsync/PostAsync is totally and utterly broken #1366

Open
davhdavh opened this issue Jun 2, 2022 · 7 comments
Open

Teams AddAsync/PostAsync is totally and utterly broken #1366

davhdavh opened this issue Jun 2, 2022 · 7 comments

Comments

@davhdavh
Copy link

davhdavh commented Jun 2, 2022

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

@ghost ghost added the Needs: Triage label Jun 2, 2022
@pschaeflein
Copy link
Contributor

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...

@elarrieux-imanage
Copy link

No updates on this issue?

@maisarissi
Copy link
Contributor

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

@andrueastman
Copy link
Member

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.

@davhdavh
Copy link
Author

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
Copy link
Author

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

@andrueastman
Copy link
Member

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

No branches or pull requests

6 participants