fs: enable chunked reading for large files in readFileHandle

[mertcanaltin]

Added chunked reading support to readFileHandle to handle files larger than 2 GiB, resolving size limitations while preserving existing functionality.

#55864

[mertcanaltin]

I wonder if I should apply this sorting in fs.js, right now I just did it for promises,

in addition there are still places that use ERR_FS_FILE_TOO_LARGE I did not delete this message so as not to break them

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.21%. Comparing base (4ee87b8) to head (3f6d78b).
Report is 398 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #56022      +/-   ##
==========================================
+ Coverage   88.00%   89.21%   +1.21%     
==========================================
  Files         656      663       +7     
  Lines      189000   192007    +3007     
  Branches    35995    36926     +931     
==========================================
+ Hits       166320   171302    +4982     
+ Misses      15840    13576    -2264     
- Partials     6840     7129     +289     

Files with missing lines	Coverage Δ
lib/fs.js	98.29% <ø> (+4.94%)	⬆️
lib/internal/fs/promises.js	98.25% <100.00%> (+0.69%)	⬆️

... and 254 files with indirect coverage changes

[mertcanaltin]

I removed the limit test because the limit for reading large files exceeding the GiB limit with fs.readFile has been removed.

[BridgeAR]

I added the tsc label to discuss, if we want to allow users to read such big files into memory, or if it would be better to try to point out streams instead.

[mcollina]

lgtm

[BridgeAR]

The error is removed from the promise version but it's missing the callback readFile implementation. The error itself would not be needed anymore due to that and also has to be removed.
This has to be addressed before we could land this.

We discussed in the TSC meeting that it's not a good idea to read beyond that, while it's acceptable for some cases.
We also discussed around warning when reaching that limit instead. We did not yet have consensus around it, but we'll discuss it again next week to finish the decision for that.

[gireeshpunathil]

just wondering what is the next action here - @BridgeAR

[BridgeAR]

@gireeshpunathil I believe you wanted to think about the warning again.

I kept my change request since the implementation should also include the callback version next to the warning. That's currently missing :)

[mertcanaltin]

@gireeshpunathil I believe you wanted to think about the warning again.

I kept my change request since the implementation should also include the callback version next to the warning. That's currently missing :)

thanks a lot for your comments, unfortunately I saw this place late and now I sent a commit for the callback version

[BridgeAR]

The original reason for the limit was AFAIK something about some systems on some versions not being able to read that file size. If that's the case, we'd have to handle chunking on our side. I am just not certain if that still applies or if it's a legacy issue.

Please also add the warning instead of the error.

[BridgeAR]

I believe removing this is actually a mistake :)

[BridgeAR]

That should probably be kept, no matter that we do not check for the error but for the file being read.

[gireeshpunathil]

@gireeshpunathil I believe you wanted to think about the warning again.

I kept my change request since the implementation should also include the callback version next to the warning. That's currently missing :)

I thought we are going to continue the discussion in the TSC on the necessity of the warning, as we didn't converge on that IIRC.