Skip to content

Reading commit with node 22.3.0 #1933

Closed

Description

@matteo-cristino

Using node with version 22.2.0 all works perfectly, but using version 22.3.0 (that is the latest) I got the following error when trying to read a commit:

TypeError {
  message: 'Cannot create property \'caller\' on string \'incorrect header check\'',
}

TypeError: Cannot create property 'caller' on string 'incorrect header check'
    at Object.readCommit (/home/matteo/src/slangroom/node_modules/.pnpm/[email protected]/node_modules/isomorphic-git/index.cjs:12665:16)

Activity

  1. jcubic commented on Jun 17, 2024

    @jcubic
    Member

    So this project doesn't have an original author that left the project. So right now the status is that's community driven. Which means that if you will not debug this yourself, there will need to be someone that will. Are you this person?

  2. matteo-cristino commented on Jun 18, 2024

    @matteo-cristino
    Author

    I can have a look, but I can not find anywhere the documentation saying how can I launch tests. I tried pnpm i followed by pnpm test but it breaks, am I doing something wrong?

  3. matteo-cristino commented on Jun 18, 2024

    @matteo-cristino
    Author

    I managed to have the tests working on node 20.11.1 looking at #1902, but I got an error on gitlab tests

    • Hosting Providers › GitLab › push
    • Hosting Providers › GitLab › fetch
    HttpError: HTTP Error: 401 Unauthorized
    

    For what I can understand the token has expired. I also tried to clone that repo from command line with

    git clone https://isomorphic-git-test-push:<token>@gitlab.com/isomorphic-git/test.empty
    

    (do not want to write directly the token here, but it is the one from test file) and I got

    Cloning into 'test.empty'...
    remote: HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See https://gitlab.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied
    fatal: Authentication failed for 'https://gitlab.com/isomorphic-git/test.empty/'
    
  4. jcubic commented on Jun 18, 2024

    @jcubic
    Member

    Yes, those tests are failing, I need to contact the original author to get access to that repo on GitLab.

  5. mojavelinux commented on Jun 23, 2024

    @mojavelinux
    Contributor

    I'm seeing this issue too. I'm getting it when using statusMatrix, but I have no idea where the error is actually coming from. I've yet to be able to create a reproducer in isolation.

    UPDATE: Obviously, the reported error is when it is trying to set the caller property on a string. What I can't figure out is how that string error is being thrown, which is the real underlying issue.

  6. mojavelinux commented on Jun 23, 2024

    @mojavelinux
    Contributor

    Here's the actual source of the error:

         Error: incorrect header check
          at Object.inflate (node_modules/pako/lib/inflate.js:389:29)
          at inflate (node_modules/isomorphic-git/index.cjs:2669:12)
          at _readObject (node_modules/isomorphic-git/index.cjs:3156:39)
    
    

    It appears that something changed in Node.js that is breaking pako's inflate function.

    UPDATE: The problem seems to be with the creation of repositories, not reading them. After isomorphic-git creates the repository, even canonical git reports "incorrect header check". So the issue may not be with the inflate function but rather elsewhere.

  7. mojavelinux commented on Jun 23, 2024

    @mojavelinux
    Contributor

    If we switch from pako to the zlib.deflate provided by Node.js, it works. So there seems to be a problem with pako's deflate method when running on Node.js 22.3.0.

    I wish isomorphic-git would use zlib from Node.js when running in Node. Perhaps that's one way out of this.

  8. mojavelinux commented on Jun 23, 2024

    @mojavelinux
    Contributor

    Oh, I might see what the problem is. Node.js has implemented the CompressionStream class from the web. So now it gets selected (instead of pako). But it's not doing the same work as what isomorphic-git expects and is thus creating a corrupt zlib-compressed object. If I comment out that check and force it to use pako, then it works.

  9. mojavelinux commented on Jun 23, 2024

    @mojavelinux
    Contributor

    What we probably need is a new release of isomorphic-git that updates the following method to force Node.js to use pako.

    function testCompressionStream() {
      try {
        const cs = new CompressionStream('deflate');
        cs.writable.close();
        // Test if `Blob.stream` is present. React Native does not have the `stream` method
        const stream = new Blob([]).stream();
        stream.cancel();
        return true
      } catch (_) {
        return false
      }
    }
  10. mojavelinux commented on Jun 23, 2024

    @mojavelinux
    Contributor

    Actually, isomorphic-git uses Node.js' CompressionStream class when using Node.js 22.2.0. For some reason, using it under 22.3.0 causes it to create a corrupt result. So perhaps there is a bug in Node.js here.

    Still, I think Node.js should use pako or even zlib instead of CompressionStream.

  11. jcubic commented on Jun 23, 2024

    @jcubic
    Member

    BTW: I disabled the GitLab tests, so now the tests should work fine. I also got contact from the original author, he gave me creds for GitLab, but I haven't checked them yet.

  12. mojavelinux commented on Jun 23, 2024

    @mojavelinux
    Contributor

    Note that the reason this isn't a problem with inflate is because isomorphic-git always uses pako in that case. See https://github.com/isomorphic-git/isomorphic-git/blob/main/src/utils/inflate.js#L5

  13. mojavelinux commented on Jun 23, 2024

    @mojavelinux
    Contributor

    I'm rather confident this is a bug in Node.js 22.3. Consider the following code:

    ;(async () => {
      console.log(await deflate('deflate me'))
    })()
    
    async function deflate(buffer) {
      const cs = new CompressionStream('deflate')
      const c = new Blob([buffer]).stream().pipeThrough(cs)
      return new Uint8Array(await new Response(c).arrayBuffer())
    }
    
    

    We expect the following result:

    Uint8Array(18) [
      120, 156, 75,  73, 77, 203, 73,
       44,  73, 85, 200, 77,   5,  0,
       21,  96,  3, 200
    ]
    

    We get the same result with both pako.deflate and zlib.deflate too.

    However, with Node.js 22.3.0, we get this instead:

    Uint8Array(8192) [
       47,   0,   0,   0,   0,   0,   0,   0,  13,  10,   0,   0,
        0,   0,   0,   0,  72,  84,  84,  80,  47,  49,  46,  49, ...
    ]
    

    So something is massively wrong.

  14. mojavelinux commented on Jun 23, 2024

    @mojavelinux
    Contributor

    I discovered a workaround that works on Node.js 22.2.0 and 22.3.0 (and probably all other Node.js versions too).

    async function browserDeflate(buffer) {
      const cs = new CompressionStream('deflate');
      const c = new Blob([buffer]).stream().pipeThrough(cs);
      return new Response(c).blob().then((b) => new Response(b).arrayBuffer()).then((ab) => new Uint8Array(ab))
    }

    Notice that we are retrieving the blob, running it through another Response object, then converting it to Uint8Array. I have no idea why this works, but it seems like the arrayBuffer returned from the Response created with the original blob stream doesn't work in Node.js 22.3.0.

  15. jcubic commented on Jun 23, 2024

    @jcubic
    Member

    There was a similar issue recently:

    When Google Chrome changed the algorithm of CompressionStream, maybe Node did the same.

  16. verreauxblack commented on Jun 24, 2024

    @verreauxblack

    I'm also seeing the same issue when I use push method.
    TypeError: Cannot create property 'caller' on string 'incorrect header check.

    But I think error occured when I used add method.

    $ git status
    error: inflate: data stream error (incorrect header check)
    error: unable to unpack 0ce8c6a6fd82131a7f89d52a80ecb4b9112271bb header
    fatal: loose object 0ce8c6a6fd82131a7f89d52a80ecb4b9112271bb (stored in .git/objects/0c/e8c6a6fd82131a7f89d52a80ecb4b9112271bb) is corrupt
    
  17. mojavelinux commented on Jun 24, 2024

    @mojavelinux
    Contributor

    Yes, I believe that is correct. It is writing garbage files to the .git/objects directory, which it subsequently fails to read.

  18. mojavelinux commented on Jun 24, 2024

    @mojavelinux
    Contributor

    I've submitted a PR.

    The reason the tests pass even when using Node.js 22.3.0 is because the global variables for compression streams were not being seen in the test environment. I've modified the Jest configuration so that they are visible, which then triggers the reported error.

  19. matteo-cristino commented on Jun 24, 2024

    @matteo-cristino
    Author

    Thank you very much for your quick and great help! ✨

    I was planning to start debugging it this week that I have more free time, anyway if I can be helpfull in some ways leave a message and I will look into it.

  20. mojavelinux commented on Jun 24, 2024

    @mojavelinux
    Contributor

    I'd still like to know why the original code doesn't work with Node.js 22.3.0. Was the code wrong and it just happened to work, or is this really a bug in Node.js 22.3.0? The workaround is portable, but it might be suppressing an underlying issue. It would also be nice to avoid the double use of Response.

  21. jcubic commented on Jun 24, 2024

    @jcubic
    Member
  22. mojavelinux commented on Jun 24, 2024

    @mojavelinux
    Contributor

    I'm not willing to go down that road, but someone else is welcome to.

  23. matteo-cristino commented on Jun 25, 2024

    @matteo-cristino
    Author

    I opened a discussion over there https://github.com/orgs/nodejs/discussions/53581 copying your test code that explained the issue perfectly, will let you known if anyone answer to me

  24. matteo-cristino commented on Jun 25, 2024

    @matteo-cristino
    Author

    They answer, it is a node issue and they are aware of it. A fix is coming in v22.4.0 in a couple of days.

    Do you want me to ask further if there is a way to be compatible with all node 22 versions or are you good with removing 22.3.0 as supported version from the engines section in the package.json?

    "engines": {
    "node": ">=12"
    },

  25. jcubic commented on Jun 25, 2024

    @jcubic
    Member

    I think that removing 22.3.0 is a good idea.

  26. luantaraschi commented on Aug 12, 2026

    @luantaraschi
    Contributor

    I reran the minimal CompressionStream('deflate') reproducer on the affected and fixed Node versions. Node 22.3.0 still returns the invalid 8,192-byte buffer, while Node 22.4.0 and 22.20.0 both return the expected 18-byte zlib stream.

    Since Node fixed this in 22.4.0 and the project had already agreed not to support the single broken 22.3.0 release, I think this issue and #1939 can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions