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

Error when creating context using M365 group GUID #1114

Closed
1 task done
JakeStanger opened this issue Feb 24, 2023 · 6 comments
Closed
1 task done

Error when creating context using M365 group GUID #1114

JakeStanger opened this issue Feb 24, 2023 · 6 comments
Assignees
Labels
area: framework ⚙ Changes in the SDK core framework code question Further information is requested

Comments

@JakeStanger
Copy link

JakeStanger commented Feb 24, 2023

Category

  • Bug

Describe the bug

When creating a context object, using an M365 group GUID, the library may throw an error:

[2023-02-24T17:07:31.303Z] System.Private.CoreLib: Exception while executing function: Orchestrator_ProvisionTeam. System.Net.Http: This instance has already started one or more requests. Properties can only be modified before sending the first request.
[2023-02-24T17:07:31.311Z] 026a09fdb3da48bc9927c47873ded560: Function 'Orchestrator_ProvisionTeam (Activity)' failed with an error. Reason: System.InvalidOperationException: This instance has already started one or more requests. Properties can only be modified before s
ending the first request.
[2023-02-24T17:07:31.313Z]    at System.Net.Http.HttpClient.CheckDisposedOrStarted()
[2023-02-24T17:07:31.314Z]    at System.Net.Http.HttpClient.set_BaseAddress(Uri value)
[2023-02-24T17:07:31.315Z]    at PnP.Core.Services.MicrosoftGraphClient.UpdateBaseAddress(String authority)
[2023-02-24T17:07:31.316Z]    at PnP.Core.Services.PnPContextFactory.InitializeContextAsync(PnPContext context, PnPContextOptions options)
[2023-02-24T17:07:31.318Z]    at PnP.Core.Services.PnPContextFactory.CreateAsync(Guid groupId, IAuthenticationProvider authenticationProvider, CancellationToken cancellationToken, PnPContextOptions options)
[2023-02-24T17:07:31.319Z]    at PnP.Core.Services.PnPContextFactory.CreateAsync(Guid groupId, IAuthenticationProvider authenticationProvider, PnPContextOptions options)
[2023-02-24T17:07:31.320Z]    at PnP.Core.Services.PnPContextFactory.CreateAsync(Guid groupId, PnPContextOptions options)
// snip

This appears to be because the library is incorrectly tracking whether the base address has been updated when creating a Graph client.

Steps to reproduce

I am not sure if the bug is specific to this case, but here's how I encountered it:

  1. Create a context factory object. In this case, as a builder service on startup and authenticated using a certificate.
  2. Using the factory, get the admin context & create a new SharePoint team site.
  3. Using the factory, get the admin context (again*) & create a team for the group associated with the site.
  4. Using the factory, get the context for the team using its GUID.

*necessary, since the operations are split across multiple orchestrated 'activity' functions.

Expected behavior

The context should be created succesfully, or return a meaningful error if provided bad input(s).

Environment details (development & target environment)

  • SDK version: 1.8.0
  • OS: Windows 11
  • SDK used in: Azure Functions app (testing locally)
  • Framework: .NET 6
  • Tooling: Jetbrains Rider
@jansenbe
Copy link
Contributor

@JakeStanger : when you create the site in step 2 you get back the PnPContext to operate on that site, at that point the site already has a Group connected to it (as that's a prereq for being able to add a Team). Why do you want to load all again using the group id?

@jansenbe jansenbe self-assigned this Feb 25, 2023
@jansenbe jansenbe added question Further information is requested area: framework ⚙ Changes in the SDK core framework code labels Feb 25, 2023
@JakeStanger
Copy link
Author

This is because steps 2-4 are all separate Azure Functions, triggered by an orchestrator. The orchestrator can provide the factory since it's automagically injected, but functions can only take/return serializable data as arguments and (I think at least) there is no guarantee field values on the class level will persist if set by functions for similar reasons.

In short, the context is disposed of at the end of each function, so a new context is required at the start of each.

@jansenbe
Copy link
Contributor

@JakeStanger : so the failure comes from https://github.com/pnp/pnpcore/blob/dev/src/sdk/PnP.Core/Services/Core/PnPContextFactory.cs#L467 and https://github.com/pnp/pnpcore/blob/dev/src/sdk/PnP.Core/Services/Core/Http/MicrosoftGraphClient.cs#L67-L74. How are you added the IPnPContextFactory to the dependency container in your function? Also, are you working with different graph backends from the same function?

note: I've just pushed a small update that only updates the base address when there's a true change, this might help already. This update will be part of the next nightly release (version 1.8.114 or higher)

image

@JakeStanger
Copy link
Author

JakeStanger commented Feb 27, 2023

How are you added the IPnPContextFactory to the dependency container in your function?

The factory is created in Startup.cs inside Configure:

builder.Services.AddPnPCore(options =>
        {
            options.PnPContext.GraphFirst = true;
            // snip - auth code
        });

This means it can get automatically injected into the orchestrator class constructor:

public sealed class Orchestrator
{
    private readonly IPnPContextFactory _contextFactory;

    public Orchestrator(IPnPContextFactory contextFactory)
    {
        _contextFactory = contextFactory;
    }

    // snip
}

We then have access to the factory from each function via the _contextFactory field.

Also, are you working with different graph backends from the same function?

Not from the same function. The previous function creates the team using the SP admin context, which I could arguably be doing in the failing function for consistency's sake anyway.

note: I've just pushed a small update that only updates the base address when there's a true change

Cool, cheers! I will try this once it has been built and I have a chance and let you know how it goes

@jansenbe
Copy link
Contributor

jansenbe commented Mar 2, 2023

@JakeStanger : did things improve with the extra check in the code?

@JakeStanger
Copy link
Author

Hey, sorry got distracted by other bits & only just tested this - it looks to have worked :)

@jansenbe jansenbe closed this as completed Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: framework ⚙ Changes in the SDK core framework code question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants