koltyakov / sp-rest-proxy

🌐 SharePoint API Proxy for local development
MIT License
172 stars 43 forks source link

sp-pnp-js batch requests failing with 400: Bad Request #42

Closed ghost closed 6 years ago

ghost commented 6 years ago

Hi,

I am building an Angular 5 application using sp-rest-proxy for local development. I am getting a 400 bad Request when trying to do a batch request using sp-pnp-js.

SharePoint Version : SharePoint Online sp-rest-proxy : latest sp-pnp-js: 3.0.4

const batch = pnp.sp.createBatch();
    array.forEach(element => {
      web.lists.getByTitle('ListName').items(element.Id).inBatch(batch).get()
        .then(data => console.log(data))
        .catch(error => console.log(error));
    });
    batch.execute().then(() => {
      console.log('All is well');
    });

Error:

Error: Error making HttpClient request in queryable: [400] Bad Request
    at new ProcessHttpClientResponseException (exceptions.js:24)
    at eval (core.js:33)
    at ZoneDelegate.invoke (zone.js:388)
    at Object.onInvoke (core.js:4745)
    at ZoneDelegate.invoke (zone.js:387)
    at Zone.run (zone.js:138)
    at eval (zone.js:858)
    at ZoneDelegate.invokeTask (zone.js:421)
    at Object.onInvokeTask (core.js:4736)
    at ZoneDelegate.invokeTask (zone.js:420)

Please let me know if you need any additional information. Thank you for your great work!

koltyakov commented 6 years ago

Hey @vpsrihari, Thanks for reporting, I'll take a look. With a manually created batch body, you can control web URLs to be a "real" for the target environment. In case of PnPjs, it takes care of constructing multipart payload and it happens that URLs correspond to proxy's, but as they proceed on the actual server they become invalid. I have an idea how to fix this.

koltyakov commented 6 years ago

Fixed and published a new version. Please check. The algorithm of changing URLs in batch multipart has been changed. So now it should work not only when the origin is the same as a proxy's origin, but also with the case of any localhost URI. Tested it with CORS and WebpackDevServer. I was able to run batch request generated with PnPjs.

ghost commented 6 years ago

Awesome! Thank you for the quick fix @AndrewKoltyakov . It now works as expected.

koltyakov commented 6 years ago

Nice! Closing the issue then.

ghost commented 6 years ago

Looks like there is another issue with batch request. Though your fix works fine for root site collection it fails for all other site collections as the requests are all modified to be sent to root site collection url and not the baseUrl set in sp-pnp-js or the site url configured in sp-rest-proxy. Please let me know if you need further information.

PS C:\sp-rest-proxy> npm run server

> simpleserver@1.0.0 server C:\sp-rest-proxy
> node ./server.js

SharePoint REST Proxy has been started on http://localhost:8080

POST: https://somesite.sharepoint.com/sites/goalsdev/2018/_api/contextinfo
Request body: {}
POST (batch): https://somesite.sharepoint.com/sites/goalsdev/2018/_api/$batch
Request body: --batch_57ee6186-86a2-4671-9df0-c6396c1454cd
Content-Type: application/http
Content-Transfer-Encoding: binary

GET https://somesite.sharepoint.com/_api/web/lists/getByTitle('SomeList')/items HTTP/1.1
accept: application/json; odata=verbose
content-type: application/json;odata=verbose;charset=utf-8
x-clientservice-clienttag: PnPCoreJS:3.0.5

--batch_57ee6186-86a2-4671-9df0-c6396c1454cd--
koltyakov commented 6 years ago

Can you provide your code sample with what you experience this? I've tested definitely not on a root site collection when it worked.

Are you creating web object with new Web('http://localhost:8080/sites/goalsdev/2018')?

Are you creating a batch from the web object?

import { loadPageContext } from 'sp-rest-proxy/dist/utils/env';
import { Web } from 'sp-pnp-js';

// loadPageContext - gets correct URL in localhost and SP environments
loadPageContext().then(async _ => { 

  const web = new Web(_spPageContextInfo.webAbsoluteUrl);
  const batch = web.createBatch();

  const list = web.getList(`${_spPageContextInfo.webServerRelativeUrl}/Lisst/ListName`);
  const entityName = await list.getListItemEntityTypeFullName();

  [1, 2, 3, 4].forEach(el => {
    list.items.inBatch(batch).add({
      Title: `${el}`
    }, entityName);
  });

  await batch.execute();
  console.log('Done');

}).catch(log);
ghost commented 6 years ago

No. I set the baseUrl of sp-pnp-js in app.component.ts as below:

environment.baseSiteUrl = 'http://localhost:8080'. This works fine for normal requests. But when I do batching, it fails.

ngOnInit() {
    // setting up sp-pnp-js
    pnp.setup({
      sp: {
        headers: {
          'Accept': 'application/json;odata=verbose'
        },
        baseUrl: `${environment.baseSiteUrl}`
      }
    });
  }

then, in the component file, I create a batch request as below:

loadFormInformation() {
    const batch = pnp.sp.createBatch();

    // getting line of business values
    // tried with pnp.sp.web as well as a new web object
    // const web = new Web(environment.baseSiteUrl);
    pnp.sp.web.lists.getByTitle(environment.FromTosListName).items
      .inBatch(batch)
      .getAs<Array<ILOB>>().then((data) => {
        this.lineOfBusinessArray = data;
        console.log(this.lineOfBusinessArray);
      }).catch((error) => {
        console.error(error);
      });
}
koltyakov commented 6 years ago

Out of the interest, why batches for this part of the code? They make no sense in the single operation of getting lists items. In the original issue description code is the same situation, no batches needed.

Anyway, will take a look with setup/baseUrl option. What is exactly in environment.baseSiteUrl variable?

ghost commented 6 years ago

For the sake of code clarity, I kept only one GET request of the batch when reporting the issue here. In my original code, I would be getting information from multiple lists in order to populate few drop downs in a form.

ghost commented 6 years ago

baseSiteUrl is a property in the environment.ts file. For local development, this refers to the proxy url (http://localhost:8080) and for Production, this would refer to the web url of the site where the application is deployed.

environment.baseSiteUrl = 'http://localhost:8080' // local development environment.baseSiteUrl = 'https://somesite.sharepoint.com/sites/goalsdev/2018' // production

koltyakov commented 6 years ago

Oh, ok. Just wanted to be sure that batches are not misused.

environment.baseSiteUrl = 'http://localhost:8080/sites/goalsdev/2018' // local development

Would be correct for sure.

Proxy resolves / as siteUrl from settings in ordinary requests. But in multipart bodies we should be more strict. And I always recommend using same paths structure.

That's why this import { loadPageContext } from 'sp-rest-proxy/dist/utils/env'; helper was created. The helper experts that WebpackDevServer is targeted it's proxy to sp-proxy as well.

ghost commented 6 years ago

Changing the baseSiteUrl did the trick @koltyakov . I always used just the localhost:8080 and it worked well for single requests. I assumed that since we already specify the sharepoint site url when starting the proxy, all subsequent requests will be based off of that url. Thank you for the great work.