Cli-microsoft365: Bug report: Large file downloading from SharePoint raise RangeError

[tobyatgithub]

Category

• [x] Enhancement
• [x] Bug
• [x] Question

Version

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

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

Expected / Desired Behavior / Question

Download the zip file (about 4.5GB) into the specified path location.

Observed Behavior

buffer.js:224
    throw err;
    ^

RangeError: "size" argument must not be larger than 2147483647
    at Function.Buffer.allocUnsafe (buffer.js:253:3)
    at Function.Buffer.concat (buffer.js:441:23)
    at Request.<anonymous> (/usr/local/lib/node_modules/@pnp/office365-cli/node_modules/request/request.js:1133:30)
    at emitOne (events.js:116:13)
    at Request.emit (events.js:211:7)
    at IncomingMessage.<anonymous> (/usr/local/lib/node_modules/@pnp/office365-cli/node_modules/request/request.js:1083:12)
    at Object.onceWrapper (events.js:313:30)
    at emitNone (events.js:111:20)
    at IncomingMessage.emit (events.js:208:7)
    at endReadableNT (_stream_readable.js:1064:12)

Steps to Reproduce

Hi guys, first time to raise an issue here. Hope I'm doing it correctly.
I was trying to make a pipeline for downloading large zip data file from sharepoint online.
I tested with smaller zip files and I worked fine.
But when I tried on a larger 4.5 GB zip file it failed.
I searched online and it seems this error comes from the js buffer which has a limitation about 2.14GB

I'm wondering will there be a fix soon? or maybe some methods to bypass this 2GB limitation?

Thank you very much!
(related stackoverflow I found: https://stackoverflow.com/questions/44994767/javascript-read-large-files-failed)

[patrick-rodgers]

I think you may have wanted to post this here: https://github.com/pnp/cli-microsoft365/issues? If so I can transfer it.

[tobyatgithub]

Hi Patrick, I think you are right! Sorry for misplacing this issue. It will be great if you can transfer it! Thank you and appreciate it :O

[juliemturner]

No worries, moving it for you now!

[waldekmastykarz]

We'll have a look at is asap. Thank you for reporting @tobyatgithub and sorry for the trouble

[waldekmastykarz]

@tobyatgithub could you give us the exact command that you're calling so that we have clear repro steps?

[tobyatgithub]

Hi!
Yes, the terminal commend is this (calling from python):

cmd = f"spo file get --webUrl '{self.__args.office365_url}' --url '{target_path}' --asFile --path '{download_path}'"

[waldekmastykarz]

@appieschot didn't you have some big files we could test this issue with?

[appieschot]

@waldekmastykarz Yes I do have some test scenario's for the move issue we are having. Tested with 10mb, 25mb, 100mb, 1gb. Those seem to download perfectly with version 3.0.0 of the CLI on windows. But after reading the issue it might has to do with the fs.readfilesync which has a 2GB buffer limit in node x64. I will try and find a 5gb file to test it with later today.

[appieschot]

A little more detail; using m365 spo file get --webUrl 'https://tenant.sharepoint.com/sites/crisismanagement' --url '/sites/crisismanagement/Docs2/Test/5GB.db' --asFile --path 'C:/temp/5GB.db'

I had the following experience:

• the node process downloads with about 66mb/s and grows accordingly to around 5.2gb (Given the 8gb limit I started killing of some other processes like teams)
• At 5.2GB memory consumption it started writing to disk
• It did fail with the following error:

internal/validators.js:85
      throw new ERR_OUT_OF_RANGE(name, `>= ${min} && <= ${max}`, value);
      ^

RangeError [ERR_OUT_OF_RANGE]: The value of "length" is out of range. It must be >= 0 && <= 2147483647. Received 5_368_709_120
    at Function.concat (buffer.js:540:5)
    at Request.<anonymous> (C:\Git\pnp\office365-cli\node_modules\request\request.js:1133:30)
    at Request.emit (events.js:223:5)
    at IncomingMessage.<anonymous> (C:\Git\pnp\office365-cli\node_modules\request\request.js:1083:12)
    at Object.onceWrapper (events.js:312:28)
    at IncomingMessage.emit (events.js:228:7)
    at endReadableNT (_stream_readable.js:1185:12)
    at processTicksAndRejections (internal/process/task_queues.js:81:21) {
  code: 'ERR_OUT_OF_RANGE'
}

Going through the code it of the request.js file it seems to fail when putting the repsone from the buffer to the body that it returns. The only thing I can come up with is to use a filestream instead of the body response using the pipe option request('http://google.com/doodle.png').pipe(fs.createWriteStream('doodle.png')) but I am not sure if that would work.

[waldekmastykarz]

Nice findings! Have you tried using the .pipe() you mentioned? Does it work as intended?

[appieschot]

Not yet, Will try today or Monday!

[appieschot]

So minor update; managed to test some more ;), but not quite there yet. We are using a custom implementation for all our requests in the request.ts file. I hacked in a .pipe() there and now I do get the file written to disk but somewhere the response is still cast, or transformed, as a string. I still get the same error (but now only after the file is saved to disk). Debugging is a bit hard since it does require at least a 2.1 GB file 😊. Will try to see if I can come up with a better approach on Monday.

[tobyatgithub]

Hi @appieschot ! How did the alternative approach go?
I'm happy as far as we can have the file written to disk!

[appieschot]

@tobyatgithub it looks like I have managed to solve the issue in this repo. It took me a bit more time than hoped for. As it turns out the request-promise-native library we are using is doing something with the body and will always try to translate the body to a string and is loading everything into memory. So after giving up on fixing that I did include the request library as that is a dependency of request-promise-native and low and behold things are making more sense now.

The whole stuff is also way better on memory consumption as it is no longer trying to download the full 2.5GB of my test file into memory. Would you have a change to test this repo against your scenario? I did manage to test with some various file sizes (from 10mb to 5GB) but before creating a PR and cleaning up a bit I would love some feedback! Thanks in advance.

@pnp/cli-for-microsoft-365-maintainers feel free to chime in as well ;-).

[tobyatgithub]

Hi @appieschot ! Silly question... how do I test this repo against my scenario?
Shall I clone the repo and paste over some already existing files? (I'm not sure how to specify versions via installation of yarn/npm)

[appieschot]

@tobyatgithub no problem! The quickest way is to clone my repo and follow the steps in our guide. Let me know if that helps!

[tobyatgithub]

@appieschot Hmm...! I think I'm doing something wrong. I got two errors when running npm test
Could you help me with that please!

What I did:

1. started docker without RUN npm i -g @pnp/office365-cli in my Dockerfile
   (I ran the pip list to ensure office365-cli
2. inside the docker terminal, I clone the git repo, and cd cli-microsoft365
3. git checkout from master to git checkout feature/bug-1796
4. running the install from the guide:

npm i
npm run build
npm test

The error message from npm test:

 8342 passing (10s)
  2 failing

  1) spo file get
       writeFile called when option --asFile is specified:

      AssertionError [ERR_ASSERTION]: false == true
      + expected - actual

      -false
      +true

      at cmdInstance.action (dist/m365/spo/commands/file/file-get.spec.js:348:17)
      at SpoFileGetCommand.handleRejectedODataJsonPromise (dist/Command.js:242:21)
      at request_1.default.getLargeFile.then (dist/m365/spo/commands/file/file-get.js:70:30)
      at <anonymous>
      at process._tickCallback (internal/process/next_tick.js:188:7)

  2) spo file get
       writeFile called when option --asFile is specified (debug):

      AssertionError [ERR_ASSERTION]: false == true
      + expected - actual

      -false
      +true

      at cmdInstance.action (dist/m365/spo/commands/file/file-get.spec.js:379:17)
      at SpoFileGetCommand.handleRejectedODataJsonPromise (dist/Command.js:242:21)
      at request_1.default.getLargeFile.then (dist/m365/spo/commands/file/file-get.js:70:30)
      at <anonymous>
      at process._tickCallback (internal/process/next_tick.js:188:7)

~Is it due to permission issue?~
-> tried run docker as admin..still get these two errors.

[appieschot]

Oh thats my bad! I never ran the tests, those are not updated yet. Could you skip the test for now so after npm run build just to the npm link. The tests are failing because I have changed the logic of saving with the --asFile parameter. I will go through the tests asap, but the code it self is correct so you can ignore the test for the first run :). Sorry for the inconvenience!

[tobyatgithub]

Hi @appieschot ! I was able to get the package installed in docker.
But when I'm using the same script that worked before, it gives this login error.
Shall I add a filename attribute to the args when I call client = O365Client(args)?

Traceback (most recent call last):
  File "download_data.py", line 67, in <module>
    client.login()
  File "/code/util/o365_cli.py", line 126, in login
    r = self.run(input=b"status")
  File "/code/util/o365_cli.py", line 115, in run
    p = subprocess.Popen(args, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
  File "/usr/lib/python3.6/subprocess.py", line 729, in __init__
    restore_signals, start_new_session)
  File "/usr/lib/python3.6/subprocess.py", line 1278, in _execute_child
    executable = os.fsencode(executable)
  File "/usr/lib/python3.6/os.py", line 800, in fsencode
    filename = fspath(filename)  # Does type-checking of `filename`.
TypeError: expected str, bytes or os.PathLike object, not NoneType

[MaxTranced]

Will the next package release include this fix?

[waldekmastykarz]

As soon as @appieschot submits a PR, we'll try to get it in as quickly as possible.

[tobyatgithub]

Hi @waldekmastykarz @appieschot ! Any news on this fix? Thank you!

[appieschot]

Hi @tobyatgithub i have a few days off in the run up to ignite, hope to work on it at the end of the week based on your replies. Sorry for the delay! Been busy remodeling our home.

[MaxTranced]

:) Hope that the remodeling is going smoothly!

Is there an ETA for the next release that includes the bugfix? Or would it be better for us to find a workaround in the meantime, @appieschot ?

[appieschot]

hey @tobyatgithub the current implementation requires that you have a filename specified if you are trying to download large files. We are thinking of making the path required if you are using the asFile, how would you feel about that? If we can get an agreement on that topic I am sure I can wrap up the PR somewhere this week :).

[waldekmastykarz]

@appieschot if I look at the current documentation, it already states that path is required when using asFile

[appieschot]

Darn, you are absolutely right! Will update tests and do PR (have to do some rework due to our latest changes but will get them in asap)

[appieschot]

Minor update; got all code working. Need to update two more tests to get it working properly. Sorry for the delay, but end of this week still sounds viable ;-).

[waldekmastykarz]

@tobyatgithub we have just merged the fix for this issue and it's available in the latest beta version v3.2. You can try it by installing the beta using: npm i -g @pnp/cli-microsoft365@next. Once again, thank you for reporting! 👏

[garrytrinder]

Great work on this @appieschot 👏🏻🚀❤️