Cannot append Uint8Array to a core
sammacbeth opened this issue · 9 comments
In node appending Uint8Array
data to a core fails, and the callback is never called:
Given the following code:
const ram = require('random-access-memory')
const Hypercore = require('hypercore')
const core = new Hypercore(ram)
core.ready(() => {
core.append(new Uint8Array(10), (err) => {
if (err) {
console.warn('append error', err);
return
}
console.log('append successful');
});
});
This code works fine in the browser, but when running in node we get the following:
req.data.copy(page, rel, start, end)
^
TypeError: req.data.copy is not a function
at RAM._write (/Users/sammacbeth/work/hyper-yjs/node_modules/random-access-memory/index.js:49:14)
at Request._run (/Users/sammacbeth/work/hyper-yjs/node_modules/random-access-storage/index.js:207:40)
at drainQueue (/Users/sammacbeth/work/hyper-yjs/node_modules/random-access-storage/index.js:246:48)
at Request._unqueue (/Users/sammacbeth/work/hyper-yjs/node_modules/random-access-storage/index.js:167:23)
at Request.callback (/Users/sammacbeth/work/hyper-yjs/node_modules/random-access-storage/index.js:172:8)
at nextTickCallback (/Users/sammacbeth/work/hyper-yjs/node_modules/random-access-storage/index.js:272:7)
at processTicksAndRejections (internal/process/task_queues.js:81:21)
This error comes from a tick, and the append callback is never called.
The source of this error is that the Uint8Array
is being treated as a Buffer
when it is not one, and this can be fixed with Buffer.from(new Uint8Array(10))
. However, the error handling here is problematic - it cannot be caught or detected, and the error message appears in the storage layer.
random-access-*
assumes data to be passed-in as Buffer's rather than Uint8Array.
What do you think is the better approach: Document everything in hyper-* assumes Buffer or changing the code to assume everything is Uint8Array?
Coercing uint8arrays to buffers inside append in hypercore is prob easy
@sammacbeth would that be ok for what you are doing?
Otherwise .copy can be replaced with .set?
@emilbayes across 8 billion repos tho
My concern is largely around error reporting - the error is currently not surfaced to the caller, making it difficult to initially debug the cause of the error.
I see two options:
- Fail fast - throw an error when non-buffers are appended to binary cores - this forces clients to convert to buffer.
- Do Buffer conversion automatically when uint8array is passed to
append
. I'm not sure what the perf cost of type check and conversion is, and if this would be an issue? I guess this can't be worse than existing conversions for other value encodings?
@sammacbeth you can convert them for "free" using buf = Buffer.from(uint8.buffer, uint8.byteOffset, uint8.byteLength)
In that case it could probably be handled by the binary codec here: https://github.com/mafintosh/codecs/blob/master/index.js#L13
I just added support for that right there actually!
(if you reinstall it should get picked up)