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

Decoding of serialized object fails when there are about 70,000 keys or more #147

Open
rotemdan opened this issue Oct 6, 2024 · 3 comments

Comments

@rotemdan
Copy link

rotemdan commented Oct 6, 2024

System info

  • msgpackr version: 1.11.0
  • node version: v22.9.0
  • OS: Windows 11 x64

Reduced test case

import { encode, decode } from 'msgpackr'

export function test() {
	let obj: any = {}

	for (let i = 0; i < 70000; i++) {
		obj['x' + i] = true
	}

	const encoded = encode(obj)

	const decoded = decode(encoded)

	console.log(decoded)
}

test()

decode(encoded) fails with error:

Error: Data read, but end of buffer not reached 
{"x0":true,"x1":true,"x2":true,"x3":true,"x4":true,"x5":true,"x6":true,"x7":true,"x8":true,"x9":true

If reducing loop to 60,000 iterations (object keys), there is no failure on decoding, meaning the issue is likely related to the size of the object.

@kriszyp
Copy link
Owner

kriszyp commented Oct 6, 2024

As stated in the docs: https://github.com/kriszyp/msgpackr?tab=readme-ov-file#options, if you want more than 64K of properties, you need to use the variableMapSize option.

@kriszyp kriszyp closed this as completed Oct 6, 2024
@rotemdan
Copy link
Author

rotemdan commented Oct 6, 2024

Thanks. I assumed that the default options work with any input. I was testing several libraries so I assumed this one behaves the same.

Other libraries I tried, like msgpack-lite, don't require special options for particular inputs. I don't think having more than 64,000 keys is that unusual (especially for the kind of data that is serialized to disk).

The error message looks more like a failure then a settings issue. Maybe the error message can suggest to set the option instead?

@kriszyp kriszyp reopened this Oct 6, 2024
@kriszyp
Copy link
Owner

kriszyp commented Oct 6, 2024

Good suggestion, thank you.

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

No branches or pull requests

2 participants