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

fix: Switch to transform streams for csv-writer to avoid crash when writing huge CSVs #617

Merged
merged 1 commit into from
Jul 29, 2023

Conversation

bgandin
Copy link
Contributor

@bgandin bgandin commented Jul 28, 2023

When writing a CSV that is too large (GBs) the app crashes with a RangeError: Invalid string length.

From csv-writer's documentation:

However, if you need to keep writing large data to a certain file, you would want to create node's transform stream and use CsvStringifier, which is explained later, inside it , and pipe the stream into a file write stream.

This PR switches to a transform stream to fix the issue. Tested with a 1 GB+ CSV MissingParentRecordsReport.csv, and it seems to work fine.

@salesforce-cla salesforce-cla bot added the cla:signed CLA signed label Jul 28, 2023
@hknokh hknokh merged commit 5a301cd into forcedotcom:master Jul 29, 2023
@hknokh
Copy link
Collaborator

hknokh commented Jul 29, 2023

Hello!
Thank you very much for the contibute!

hknokh added a commit that referenced this pull request Jul 31, 2023
hknokh added a commit that referenced this pull request Jul 31, 2023
hknokh added a commit that referenced this pull request Jul 31, 2023
@hknokh
Copy link
Collaborator

hknokh commented Jul 31, 2023

Hello,
I have abandoned your merge request, reverting back to the previous version. The reason for this decision is a possible issue caused by your fix, which is described in the following GitHub issue: #618.
Thank you for your understanding.
Best regards.

R0Wi pushed a commit to R0Wi/SFDX-Data-Move-Utility that referenced this pull request Aug 1, 2023
* Stream behaviour was introduced with forcedotcom#617
* We have been waiting for the csvTransformStream only
* FileStream was not fully flushed so we missed CSV records forcedotcom#618
R0Wi added a commit to R0Wi/SFDX-Data-Move-Utility that referenced this pull request Aug 1, 2023
* Stream behaviour was introduced with forcedotcom#617
* We have been waiting for the csvTransformStream only
* FileStream was not fully flushed so we missed CSV records forcedotcom#618
R0Wi added a commit to R0Wi/SFDX-Data-Move-Utility that referenced this pull request Aug 2, 2023
* Stream behaviour was introduced with forcedotcom#617
* We have been waiting for the csvTransformStream only
* FileStream was not fully flushed so we missed CSV records forcedotcom#618
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:signed CLA signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants