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

Getting exception "cannot read property 'forEach' of undefined #1017

Closed
1 of 4 tasks
derekesh opened this issue Jan 17, 2020 · 30 comments · Fixed by #1042
Closed
1 of 4 tasks

Getting exception "cannot read property 'forEach' of undefined #1017

derekesh opened this issue Jan 17, 2020 · 30 comments · Fixed by #1042

Comments

@derekesh
Copy link

derekesh commented Jan 17, 2020

Thank you for reporting an issue, suggesting an enhancement, or asking a question. We appreciate your feedback - to help the team understand your
needs please complete the below template to ensure we have the details to help. Thanks!

Please check out the Docs to see if your question is already addressed there. This will help us ensure our documentation covers the most frequent questions.

Category

  • Enhancement
  • Bug
  • Question
  • Documentation gap/issue

Version

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

Please specify what version(s) of SharePoint you are targeting: [ SPO ]

If you are not using the latest release, please update and see if the issue is resolved before submitting an issue.

Expected / Desired Behavior / Question

Our app is a React application which we're hosting on our site via a custom webpart. We are trying to retrieve items from a list on the same site via the following code:

let items: IItems = sp.web.lists.getByTitle(listName).items;
    if (expandAttachments) {
      items = items.expand("AttachmentFiles");
    }
    if (orderBy) {
      items = items.orderBy(orderBy, asc);
    }
    if (top) {
      items = items.top(top);
    }
    return items.get().catch(SpApiService.handleError);

Observed Behavior

We are getting a JS exception when trying to execute the request:

TypeError: Cannot read property 'forEach' of undefined
    at mergeHeaders (net.js:6)
    at SPHttpClient.<anonymous> (sphttpclient.js:23)
    at step (tslib.es6.js:99)
    at Object.next (tslib.es6.js:80)
    at tslib.es6.js:73
    at new Promise (<anonymous>)
    at __awaiter (tslib.es6.js:69)
    at SPHttpClient.fetch (sphttpclient.js:15)
    at pipeline.js:162
    at new Promise (<anonymous>)

Steps to Reproduce

We are getting this exception with multiple versions of the library.

Any suggestions or help would be greatly appreciated!

@patrick-rodgers
Copy link
Member

Can you share the set of imports you are using?

@derekesh
Copy link
Author

These are all of the third-party imports we're using in this component:
import { Observable } from "rxjs";
import { sp } from "@pnp/sp";

@koltyakov
Copy link
Member

koltyakov commented Jan 17, 2020

@derekesh, you missed some imports. V2 uses selective imports, more details. I'd recommend import { sp } from '@pnp/sp/presets/all'; for a transition, then in case of a need to reduce bundle size refactor to selective partial imports.

@derekesh
Copy link
Author

derekesh commented Jan 17, 2020

@koltyakov I've updated the imports. I've also added a header to the sp.setup call because, based on the error, it looks like some headers (I'm not clear which ones) are not being populated. After doing both of thise things, the code looks like this:

import { IItems, sp } from "@pnp/sp/presets/all";

sp.setup({
      sp: {
        headers: {
          Accept: "application/json;odata=verbose"
        },
        baseUrl: {absoluteUrl}
      }
    })

let items: IItems = sp.web.lists.getByTitle(listName).items;
    if (expandAttachments) {
      items = items.expand("AttachmentFiles");
    }
    if (orderBy) {
      items = items.orderBy(orderBy, asc);
    }
    if (top) {
      items = items.top(top);
    }
return items.get().catch(SpApiService.handleError);

I'm still receiving the error, with the following stack trace:

TypeError: Cannot read property 'forEach' of undefined
    at Object.mergeHeaders (net.js:6)
    at SPHttpClient.<anonymous> (sphttpclient.js:25)
    at step (tslib.es6.js:99)
    at Object.next (tslib.es6.js:80)
    at tslib.es6.js:73
    at new Promise (<anonymous>)
    at Object.__awaiter (tslib.es6.js:69)
    at SPHttpClient.fetch (sphttpclient.js:17)
    at pipeline.js:162
    at new Promise (<anonymous>)

@patrick-rodgers
Copy link
Member

The message is coming from one of these mergeHeaders lines - but mergeHeaders should handle the case where the source is null or undefined. Will have to dig in and see how this is happening as on initial inspection I don't see it. This is also a code path that is called a TON so it shouldn't have slipped past - but anything is possible.

@patrick-rodgers
Copy link
Member

This might be related somehow to #1016 - not sure but until that is addressed anything 2.0.1 is suspect. Can you try falling back to 2.0.0 and see if things work?

@derekesh
Copy link
Author

derekesh commented Jan 21, 2020

I am still experiencing the same errors after rolling back to 2.0.0:

Cannot read property 'forEach' of undefined
    at mergeHeaders (net.js:6)
    at SPHttpClient.<anonymous> (sphttpclient.js:23)
    at step (tslib.es6.js:99)
    at Object.next (tslib.es6.js:80)
    at tslib.es6.js:73
    at new Promise (<anonymous>)
    at __awaiter (tslib.es6.js:69)
    at SPHttpClient.fetch (sphttpclient.js:15)
    at pipeline.js:162
    at new Promise (<anonymous>)

I have a support ticket open with MS and am working through that with them as well.

@patrick-rodgers
Copy link
Member

I am not able to reproduce this error. The code I used:

// because I am testing in node
import { SPFetchClient } from "@pnp/nodejs";

import { sp } from "@pnp/sp";
import "@pnp/sp/webs";
import "@pnp/sp/lists";
import "@pnp/sp/items";
import { IItems } from "@pnp/sp/items";

sp.setup({
  sp: {
    // only because I am testing in node
    fetchClientFactory: () => {
      return new SPFetchClient(settings.testing.sp.url, settings.testing.sp.id, settings.testing.sp.secret);
    },
    headers: {
      Accept: "application/json;odata=verbose",
    },
  },
});

let items: IItems = sp.web.lists.getByTitle("OrderByList").items;

if (true) {
  items = items.expand("AttachmentFiles");
}
if (true) {
  items = items.orderBy("Title");
}
if (true) {
  items = items.top(10);
}

const w = await items.get();

console.log(JSON.stringify(w, null, 2));

That works for me without issue. Do you have any other details you can share or anything you are doing that might affect things outside the code you have posted? At this time I don't think the issue is within the library - but certainly want to find out if it is.

@derekesh
Copy link
Author

derekesh commented Jan 28, 2020

A few bits more of information. I am receiving this error with both this library and the previous, deprecated version of the library "sp-pnp-js". I seem to be getting it regardless of version.

I implemented a custom SPFx extension that calls the list using SP.js directly, and was able to get results as expected.
`
var itemArray = [];

const context: SP.ClientContext = new SP.ClientContext(this.context.pageContext.web.absoluteUrl);

var list = context.get_web().get_lists().getByTitle("Company news");

var camlQuery = new SP.CamlQuery();

camlQuery.set_viewXml('<View><RowLimit>100</RowLimit></View>');    

const items = list.getItems(camlQuery);

context.load(items);   

context.executeQueryAsync((sender: any, args: SP.ClientRequestSucceededEventArgs): void => {          

    var itemEnumerator = items.getEnumerator();

    while(itemEnumerator.moveNext()){

        var item = itemEnumerator.get_current();

        var id = item.get_item("ID");

        var title = item.get_item("Title");

        itemArray.push(id + ": " + title);

    }

}
`

I'm continuing to work this with MS support and will post any updates.

@patrick-rodgers
Copy link
Member

In what browser are you seeing the error?

@derekesh
Copy link
Author

I'm seeing this in both Chrome and FF. In Chrome, the error is "TypeError: Cannot read property 'forEach' of undefined". In FF, the error is "TypeError: 'temp.headers is undefined'"

@derekesh derekesh reopened this Jan 28, 2020
@patrick-rodgers
Copy link
Member

The only thing I can think is that you are somehow ending up with a source var passed to the method that is not undefined, not null, but also not a valid header init. Can you debug at that point and see if some weirdness is happening?

@derekesh
Copy link
Author

It looks like the values of both "target" and "source" parameters at line 4 are empty JSON objects { }. It looks like the value of the "temp" is a new Request object that is empty.

temp: newRequest
proto: Object
this: undefined

Hopefully that helps?

@patrick-rodgers
Copy link
Member

Are you setting somewhere the headers collection as empty? I don't see where we do that. I guess we can add a check for an empty headers collection in addition to it being null. At a certain point we can only check for so many bad inputs...

@derekesh
Copy link
Author

derekesh commented Jan 31, 2020

No, we are not modifying the headers collection in any way. I'm not too familiar with whats going on behind the scenes, but could that collection be something that's impacted by the response from SharePoint? We deployed the same build of our custom webpart to two different sharepoint sites, one which is having issues and one is not. I'm following up on that with MS support to see if there are configuration differences in the site collections that may be having an impact.

@patrick-rodgers
Copy link
Member

I can't think how the sites are setup would affect the headers collection in the library. This is also a code path called billions of times a month and this issue hasn't come up before. Have you done different setup calls in the two deployed packages?

patrick-rodgers added a commit to patrick-rodgers/pnpjs that referenced this issue Jan 31, 2020
@patrick-rodgers
Copy link
Member

I have gone ahead and added another check in the mergeHeaders method that should prevent this issue.

@derekesh
Copy link
Author

derekesh commented Feb 3, 2020

I added this check to the library code and tried running it. After adding it, the error went away and the requests were being made. However, it appears that the first request using the library is successful, and then for the second and subsequent requests, the "content-type: application/json" request header is being left off of the request. All subsequent requests come back with XML data, and an error is then being thrown when trying to call JSON.parse().

Is there something about the added check that may prevent the content-type header from being added in the future, or another change going out with this one that may ensure the content-type header is always added?

@patrick-rodgers
Copy link
Member

Can you provide a repo that shows these issues? I am unable to reproduce them locally and am having to guess at what you are doing so would need the code that shows what is breaking. If I run the code you have provided it works as expected as I wrote above and I am unable to recreate an issue with "subsequent requests".

@derekesh
Copy link
Author

derekesh commented Feb 4, 2020

Unfortunately the repo is a private repo I can't share, but I can share the contents of the .ts file in which we use the library. This file is the only file in the repo in which the library is used. Could the issue be that we are making multiple calls to the library simultaneously?

import "@pnp/sp/lists/web";
import { IItems, IList, sp } from "@pnp/sp/presets/all";
import "@pnp/sp/webs";
import { Observable } from "rxjs";
import { ISpAttachment } from "../interfaces/ISpAttatchment";
import { TimeUnits } from "../types/CachingTypes/TimeUnits";
import { SpUser } from '../types/SpUser';
import CachingService from "./CachingService";

class SpApiService {

public static initialize(absoluteUrl: string) {
sp.setup({
sp : {
baseUrl : absoluteUrl
}
});
}

public static getCurrentUser(): Promise {
return sp.web.currentUser.get() as Promise;
}

/**

  • Builds items query
  • @param listName Name of the list to be queried
  • @param expandAttachments (Optional) Expand items if items have attachments
  • @param orderBy (Optional) Field on item in which to order on
  • @param asc (Optional) Sort ascending boolean. Descending by default
  • @param top (Optional) Number of items to retrieve
    */
    private static buildQuery = (
    listName: string,
    expandAttachments: boolean = false,
    orderBy: string = "",
    asc: boolean = false,
    top: number = 0,
    ): IItems => {
let i: IItems = sp.web.lists.getByTitle(listName).items;

if (expandAttachments) {
  i.expand("AttachmentFiles");
}

if (orderBy && top) {
  i = i.orderBy(orderBy, asc).top(top);
} else if (orderBy) {
  i = i.orderBy(orderBy, asc);
} else if (top) {
  i = i.top(top);
}

return i;

};

public static getListItems = (listName: string, orderBy: string = "", asc: boolean = false, top: number = 0): Promise => {
return SpApiService.buildQuery(listName, false, orderBy, asc, top)
.get()
.catch(SpApiService.handleError);
};

/**

  • Get SharePoint list items from a specific list
  • @param listName Name of list to be queried
  • @param cacheDuration (Optional) Length data should be cached
  • @param timeUnit (Optional) Unit of time (second, minute, hour, day) for cache
  • @param orderBy (Optional) Field on item in which to order on
  • @param asc (Optional) Sort ascending boolean. Descending by default
  • @param top (Optional) Number of items to retrieve
    */
    public static async getListItemsAndCache(
    listName: string,
    cacheDuration?: number,
    timeUnit?: TimeUnits,
    orderBy: string = "",
    asc: boolean = false,
    top: number = 0,
    ): Promise {
    return CachingService.getObjectByKey(
    listName,
    (): Promise =>
    SpApiService.buildQuery(listName, false, orderBy, asc, top)
    .get()
    .catch(SpApiService.handleError),
    cacheDuration || 15,
    timeUnit || TimeUnits.minutes,
    );
    }

/**

  • Build observable that returns data from cache/api
  • @param listName Name of list to be queried
  • @param cacheDuration (Optional) Length data should be cached
  • @param timeUnit (Optional) Unit of time (second, minute, hour, day) for cache
  • @param orderBy (Optional) Field on item in which to order on
  • @param asc (Optional) Sort ascending boolean. Descending by default
  • @param top (Optional) Number of items to retrieve
    */
    public static getListItemsAndObserve = (
    listName: string,
    cacheDuration?: number,
    timeUnit?: TimeUnits,
    orderBy: string = "",
    asc: boolean = false,
    top: number = 0,
    ): Observable => {
    return CachingService.getObjectByKeyAndObserve(
    listName,
    (): Promise =>
    SpApiService.buildQuery(listName, false, orderBy, asc, top)
    .get()
    .catch(SpApiService.handleError),
    cacheDuration || 15,
    timeUnit || TimeUnits.minutes,
    );
    };

public static getListItemById = (listName: string, id: number): any => {
return sp.web.lists.getByTitle(listName).items.getById(id);
};

/**

  • Get list items with attachments
  • @param listName Name of list to be queried
  • @param cacheDuration (Optional) Length data should be cached
  • @param timeUnit (Optional) Unit of time (second, minute, hour, day) for cache
  • @param orderBy (Optional) Field on item in which to order on
  • @param asc (Optional) Sort ascending boolean. Descending by default
  • @param top (Optional) Number of items to retrieve
    */
    public static async getListItemsWithAttachments(
    listName: string,
    cacheDuration?: number,
    timeUnit?: TimeUnits,
    uncached: boolean = false,
    orderBy: string = "",
    asc: boolean = false,
    top: number = 0,
    ): Promise<T[]> {
    if (uncached) {
    return SpApiService.buildQuery(listName, true, orderBy, asc, top)
    .get()
    .catch(SpApiService.handleError);
    } else {
    return CachingService.getObjectByKey<T[]>(
    listName,
    (): Promise<T[]> =>
    SpApiService.buildQuery(listName, true, orderBy, asc, top)
    .get()
    .catch(SpApiService.handleError),
    cacheDuration || 15,
    timeUnit || TimeUnits.minutes,
    );
    }
    }

/**

  • Build observable that returns data from cache/api with attachments
  • @param listName Name of list to be queried
  • @param cacheDuration (Optional) Length data should be cached
  • @param timeUnit (Optional) Unit of time (second, minute, hour, day) for cache
  • @param orderBy (Optional) Field on item in which to order on
  • @param asc (Optional) Sort ascending boolean. Descending by default
  • @param top (Optional) Number of items to retrieve
    */
    public static getListItemsWithAttachmentsAndObserve(
    listName: string,
    cacheDuration?: number,
    timeUnit?: TimeUnits,
    orderBy: string = "",
    asc: boolean = false,
    top: number = 0,
    ): Observable<T[]> {
    return CachingService.getObjectByKeyAndObserve<T[]>(
    listName,
    (): Promise<T[]> =>
    SpApiService.buildQuery(listName, true, orderBy, asc, top)
    .get()
    .catch(SpApiService.handleError),
    cacheDuration || 15,
    timeUnit || TimeUnits.minutes,
    );
    }

public static addListItem(listName: string, item: any): void {
const list: IList = sp.web.lists.getByTitle(listName);
if (!list) {
console.log("Failed to add to list", listName, "list could not be retrieved.");
return;
}
list.items.add(item).catch((err: any) => {
console.log("Failed adding item to list", listName, err);
});
}

private static handleError(err: any): any {
console.log(err);
return [];
}
}

export default SpApiService;

@koltyakov
Copy link
Member

@derekesh,

I'm wondering, while you're using @pnp/sp/presets/all preset, why that additional partial imports?

@derekesh
Copy link
Author

derekesh commented Feb 4, 2020

I believe those are leftover imports from previous tests. I have removed them, and am still seeing the error:

SyntaxError: Unexpected token < in JSON at position 0
at JSON.parse ()
at parsers.js:22

In the first request the library makes, the "accept" and "content-type" headers are set as:
accept: application/json
content-type: application/json;odata=verbose;charset=utf-8

In all subsequent requests, the "content-type" header isn't set, and the "accept" header is:
accept: /

@derekesh
Copy link
Author

derekesh commented Feb 4, 2020

I did a bit more experimenting, and this seems to resolve the issue:

export function mergeHeaders(target, source) {
if (objectDefinedNotNull(source)) {
var temp = new Request("", { headers: source });
if (temp && temp.headers) {
temp.headers.forEach(function (value, name) {
target.append(name, value);
});
} else {
target.append("accept", "application/json");
target.append("content-type", "application/json;odata=verbose;charset=utf-8");
}
}
}

I'm not exactly sure what's going on here in depth so I'm not sure if the above is correct, but it might be a lead?

@patrick-rodgers
Copy link
Member

The problem is that this library makes billions of requests a month and you're the only one reporting this issue - and you can't share a repo we can clone and see the issue.

Making the change you indicate in mergeHeaders isn't the right way to handle this. Those headers are set in the client if they are not already present.

What I would recommend is stripping out all the other code and see if the PnPjs requests work as you expect. Can you run the code shared without issue? Then start adding back in your code and see where the problem pops up. Perhaps you are having some issue because you never appear to await the result of the actions you are performing, very hard to say.

It also seems like you are making changes within our published library - have you made other changes? Editing the library directly isn't something we can support.

It isn't that we don't want to help - but we are able to run the exact code you are sharing without a problem so not sure how to even understand what the issue might be.

@derekesh
Copy link
Author

derekesh commented Feb 5, 2020

I have not made any changes to the published library, I was just attempting to verify the changes you made for the next release before it came out. I can call the library by itself successfully without any other client code, but I can also use the library on one site successfully and not the other. The sppkg deployed to both sites is identical, but doesn't work one one site but does on the other. I even tried the deprecated version of the library (sp-pnp-js) and am getting the same error. It also looks like it may be a caching issue, so I removed all parts of our code that use browser caching and am still seeing the issue. I also tried initializing the headers in sp.setup, but am still seeing the issue.

Are awaits absolutely necessary? I can change the code to never use .then statements, but this will take some effort.

Are there any settings for SP site collections that the library may be dependent on? I've had multiple sessions with MS, and they are confident the settings and contents of both sites are identical. They've also verified in their server logs that the requests are being handled as expected. I've also verified that making these calls directly with SP.js is succeeeding.

I'm about out of things to try inside the bounds of using this library, so any other suggestions you may have would be helpful. Otherwise, we'll have to remove our dependency on this library.

@patrick-rodgers
Copy link
Member

Using await isn't necessary, but for example in the public static addListItem(listName: string, item: any): void method you posted you just fire the add item call and never wait for it to complete. Hard to say what your other code might be doing.

Here is where I am stuck:

  • The library is used successfully millions of times a day to make calls to SPO without this issue appearing
  • The library works for you in one site but not another
  • The library works for you without your other code
  • You can't provide a clonable repo that we can see exactly where things are failing or steps to reproduce the error
  • You are threatening "if you can't guess what my issue is then I will stop using your library"

I am not sure how we can better help you than all of the support we have given in this thread. At a certain point we don't have more to offer if you can't provide a clear way for us to reproduce the error, step by step. The fact you have a package that works fine in one site but not anothe would point to an issue with the code/configuration in the package, not the library.

Unless you can provide a way for us to explicitly reproduce this issue I don't have more to offer at this time.

@derekesh
Copy link
Author

derekesh commented Feb 6, 2020

I too am a software developer, and understand the difficulties and frustrations of trying to diagnose the issues of 1 user without being able to see what is going on in their specific environment. We're using the library in our production environment to facilitate more than 1 million calls to SharePoint a day, and understand the issue probably in the configuration of the sites its not working on and not in the library itself. I'm sorry if you took my comment of moving away from the library as a threat- it certainly wasn't meant to be. Unfortunately, if we're unable to diagnose the issue in our end and you and your team have no other suggestions, our only option is to move away from the library. We don't do this lightly as we know the library has been thoroughly tested and verified, but we don't really have another choice. I just wanted to make sure that we really were out of other options before we move that way.

Thank you for all of your help through this up to this point, my team and I do appreciate it. I'll close the ticket.

@derekesh derekesh closed this as completed Feb 6, 2020
@patrick-rodgers
Copy link
Member

Fair enough, though I would question the thought process that you are using the library for millions of requests successfully, it doesn't work in some cases, and that is leading you to drop using the library rather than determining what the actual issue is. I hope you are able to figure out a path forward that works for you.

@derekesh
Copy link
Author

derekesh commented Feb 6, 2020

I agree that determining the actual issue would be by far the best solution, but three weeks of analysis haven't turned up the root issue and we have other high-priority work that is being blocked by this. We're unfortunately at the point where we can no longer afford to continue with the diagnostic approach. Again, thank you for all of your assistance through this.

@github-actions
Copy link

This issue is locked for inactivity or age. If you have a related issue please open a new issue and reference this one. Closed issues are not tracked.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants