Pnpjs: Graph Batch request are send with absolute urls instead of relative urls

Created on 19 Oct 2018  路  5Comments  路  Source: pnp/pnpjs

Category

  • [ ] Enhancement
  • [ x] Bug
  • [ ] Question
  • [ ] Documentation gap/issue

Version

Please specify what version of the library you are using: [ 1.2.3 ]

Please specify what version(s) of SharePoint you are targeting: [ Online, Modern Workbench]

Expected / Desired Behavior / Question

I'm loading skills for multiple users by id with a batch request. Doing this by ID because I only want the skills from a few people which I found with the /me/people graph query. My code looks as followed:

let batch = new GraphBatch();
for (let user of users) {
    let view = new GraphQueryableCollection(graph.users.getById(user.id), "skills");
    view.usingCaching().inBatch(batch).get().then((skills: string[]) => {
        // doing something with the skills here
    }
}
await batch.execute();

Observed Behavior

In my network tab I can see that the batch request is send, however I am getting a 500 internal server error. In the request payload I can see that the url which is send is an absolute url. (https://graph.microsoft.com/v1.0/users//skills) This is sadly not working with the graph api. They are expecting relative urls. (users//skills) Is it possible to fix this in the GraphBatch class?

Another thing I am seeing is that the batch api is currently supporting a maximum of 20 requests in a single batch. Off course I can send my own batches of 20, but it would be awesome if the GraphBatch class would do this for me.

Steps to Reproduce

Check out the code above. This is how I produced this issue on a remote SharePoint workbench.

code investigate bug

Most helpful comment

Hi @pkmelee337 - the batching functionality in graph was written before it was GA and hasn't been reviewed since. Likely we need to update what we are doing so it works. To that end I am going to close your other issue and simply use this one to track "make graph batching work". Sound good?

All 5 comments

Hi @pkmelee337 - the batching functionality in graph was written before it was GA and hasn't been reviewed since. Likely we need to update what we are doing so it works. To that end I am going to close your other issue and simply use this one to track "make graph batching work". Sound good?

Hi @patrick-rodgers , Thanks for your reaction. Sounds great and I'm looking forward to the fixes. :-) For the time being I was able to work around it by creating my own GraphBatch implementation.

Another small addition. I found out that parsing of batch results wasn't working either. This is due to the fact that on row 162 in the graph batch,ts file, (in the _parseResponse) function the result is set like this:

parsedResponses[responseId] = new Response(null, {
    headers: response.headers,
    status: response.status,
});

This isn't making any sense, because the body of the 'sub'response is never set. When I do set the json body the ODataParse is going nuts because it does expect a stringified json, which let to the following code which works:

parsedResponses[responseId] = new Response(JSON.stringify(response.body), {
    headers: response.headers,
    status: response.status,
    statusText: response.statusText
});

And another small bug I found. For rertrieving user photos I am also using graph batching with the usingCaching parameter:

let view = new GraphQueryableInstance(graph.users.getById(person.id), `/photos/${size}/$value`);
view.usingCaching({
    expiration: util.dateAdd(new Date(), "day", 1),
    key: `userPhoto-id-${person.id}`,
    storeName: "local"
}).inBatch(batch).get().then((base64Result: string) => {
});

The batch request are working with my own GraphBatch implementation (copied from batch.ts with the fixed mentioned above). The caching is working also, however, when there are no batch requests left because they are all cached, it's still trying to send an empty batch request which fails because it is empty. Would it be possible to create a check on this? :-)

From this thread I extracted these tasks:

  • [X] Empty batches should not execute
  • [X] Response body was not being included
  • [ ] Limit to 20 requests/batch automatically
  • [X] Urls need to be relative inside the batch

I have addressed all of the checked items in PR #323. I am going to close this and create a single new issue for limiting the batches to 20 per http request.

Was this page helpful?
0 / 5 - 0 ratings