Skip to content

Conversation

niik
Copy link
Member

@niik niik commented Dec 16, 2024

Description

This should make its way into dugite rather than living here in Desktop but adding it here for now to be able to incorporate it into a hotfix.

As part of the work to enable large git output I lifted the previous 1Mb limit on Git output (desktop/dugite#596) and set it to Infinity. The problem with that occurs when a Git command invoked with a string encoding spits out more than the maximum string length. I thought that in that case the error thrown would get picked up by execFile and passed on into the error handler but it's not. Instead it gets thrown in an asynchronous callback that we can't catch (other than in the uncaughtException process handler).

Instead we'll check if the encoding is a "readable encoding" (i.e. not resulting in a buffer) and set the max length to kStringMaxLength which is currently 536870888 bytes. Should a Git command spit out more than this Node will throw an exception which will end up in execFile's error callback and then we can handle it gracefully.

I've also added some extra debugging here which will be useful for us going forward. By including the operation name in the execution message we should be able to figure out which operations are candidates for using buffer encoding.

Screenshots

Release notes

Notes:

Include operation name when maxBuffer is exceeded
@niik niik requested review from sergiou87 and tidy-dev December 16, 2024 10:56
Copy link
Contributor

@tidy-dev tidy-dev left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@sergiou87 sergiou87 left a comment

Choose a reason for hiding this comment

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

Wow nice catch!! 🤯 :shipit:

@niik niik merged commit af4cd4c into development Dec 16, 2024
8 checks passed
@niik niik deleted the string-theory branch December 16, 2024 13:12
@niik niik mentioned this pull request Dec 16, 2024
@WWJ30
Copy link

WWJ30 commented May 27, 2025

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.

4 participants