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 RangeError for input buffers #248

Closed
wants to merge 1 commit into from
Closed

fix RangeError for input buffers #248

wants to merge 1 commit into from

Conversation

zeke
Copy link
Member

@zeke zeke commented Apr 28, 2024

Resolves #247

This PR was written by the language model gods. Here's what they have to say about the reasoning behind the fix:


The error "RangeError: Maximum call stack size exceeded" in the original function arises due to the use of String.fromCharCode.apply(null, new Uint8Array(bytes)). This method attempts to convert a Uint8Array into a string by applying String.fromCharCode to each byte in the array. However, when the Uint8Array is large, this approach leads to a very large number of arguments being passed to String.fromCharCode via the apply method, which exceeds the JavaScript engine's maximum call stack size.

JavaScript has a limit on the number of arguments that can be passed to a function, and exceeding this limit results in a stack overflow error, as seen here. This is particularly problematic with large binary data like images, where the byte array can be very large.

The refactored bytesToBase64 function avoids this issue by iteratively building a string from the Uint8Array using a loop. In each iteration, it converts a single byte to a character using String.fromCharCode and appends this character to a string. This method does not involve passing a large number of arguments to a function at once, thus it stays within the call stack limit and avoids the error.

This approach is more efficient in terms of memory usage and avoids the call stack limit issue, making it suitable for handling large data arrays.

@zeke zeke requested a review from a team April 28, 2024 06:38
@zeke zeke changed the title fix rangeerror for input buffers fix RangeError for input buffers Apr 29, 2024
@zeke
Copy link
Member Author

zeke commented May 1, 2024

cc @mattt @aron can you take a peek at this?

Copy link
Contributor

@aron aron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure this is the right fix here. I'm curious why your example code in the ticket isn't being uploaded as a file, will reply on the ticket.

const uint8Array = new Uint8Array(bytes);
let binaryString = "";
for (let i = 0; i < uint8Array.length; i++) {
binaryString += String.fromCharCode(uint8Array[i]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been a while since I looked at JS string performance but this change makes me nervous. I believe JS strings are immutable so each time we add a character here it's making a copy of the string leaving the old one to be garbage collected.

If the issue here is that the the number of bytes passed to Uint8Array is too large, then I have a feeling this is potentially going to consume a lot of memory. An alternative would be to build an array and use join() but as this implementation was supposed to be a quick "good enough" hack, at this point I think we'd be better off re-considering vendoring in a proper cross-platform base64 lib and avoiding the use of btoa altogether.

@zeke
Copy link
Member Author

zeke commented May 2, 2024

Alternative is to ship #184, where "each binary input is uploaded and replaced with a URL to the created file".

That's fine with me!

@aron aron closed this May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RangeError: Maximum call stack size exceeded for image buffer input
2 participants