`TypeError: this.ready is not a function` thrown from stray `this.ready()` in `LinearizedCore`
lejeunerenard opened this issue · 11 comments
Got the following error message and did some sleuthing but don't know enough about how everything works to fix it:
.../node_modules/autobase/lib/linearize.js:588
if (!this.lastUpdate) await this.ready()
^
TypeError: this.ready is not a function
at LinearizedCore.get (.../node_modules/autobase/lib/linearize.js:588:38)
at LinearizedCoreSession.get (.../node_modules/autobase/lib/linearize.js:778:34)
at Batch.getBlock (.../node_modules/hyperbee/index.js:540:35)
at Batch.getRoot (.../node_modules/hyperbee/index.js:528:24)
at processTicksAndRejections (node:internal/process/task_queues:96:5)
at async TreeIterator.open (.../node_modules/hyperbee/iterators/diff.js:81:18)
at async Promise.all (index 1)
at async DiffIterator.open (.../node_modules/hyperbee/iterators/diff.js:156:5)
My Sleuthing
I took a look at the LinearizedCore class and didn't see a ready
function and couldn't find where it would be added dynamically. So I git blamed the offending line to find it was added here:
Line 473 in f1c5ffd
and also noticed that there was a
ready
function at that time:Line 396 in f1c5ffd
Finally I found that this
ready
function was remove in this commit 02a8ed5.Sounds like it could be await this.open(this.view, this.clock)
now, but I also don't know about Autobase, do you have a reproducible test case?
Ah I was basing my previous response on your links but it got further updated, so using open
is not valid
Yeah. I think this is a bug from vestigial code from refactoring. I tried replacing the this.ready()
with a modern version of the ready
function:
async get (seq, opts = {}) {
if (!this.lastUpdate) {
await this.autobase.ready()
const clock = this.clock || await this.autobase.latest()
await this._rebuild(clock)
}
...
}
And the error went away and then I got another unrelated error. Still don't know if this is an actual fix though.
I'll see if I can write a test to recreate it when I have a chance.
@LuKks Found a quite simple test to replicate the error:
test('local linearizing - get() awaits for update if not every updated before', async t => {
const output = new Hypercore(ram)
const writerA = new Hypercore(ram)
const base = new Autobase({
inputs: [writerA],
localOutput: output,
autostart: true,
eagerUpdate: false
})
await base.append('a0', await base.latest(), writerA)
t.doesNotThrow(async () => await base.view.get(0))
t.end()
})
I tried replacing the await this.ready()
my solution above and it does pass everything then. So maybe that's the solution? The check on this.lastUpdate
seems to be to ensure its set when calling _get()
lest it return null and trigger a rebuild anyways.
My fix is, make sure it updates before getting. I removed the pesky line, until there is official fix. The fix above doesn't completely fix the problem for me.
@zacharygriffee How does my fix not fix the problem for you? Are you still getting the same error with it?
No I do not get the same error. I get core doesn't find the latest appends without an update in user-land. With an update in user-land then the linearized core will have 'lastUpdate'. Correct me if I'm wrong, if I have to do the update in user-land anyways, that makes the if statement completely useless. So in a way, your fix, in my mind works, it just from the logic I have, it's irrelevant anyways. So poor wording on my end unless I'm not seeing something I should.
edit: I meant linearized core not view
My line of thinking that is why it was missed by the devs anyway, cause docs show to update before view.get, and update makes that if statement irrelevant.
You're correct that if an .update()
is required before calling .get()
then the if statements irrelevant. But I would say then an error should be thrown when this.lastUpdate
isn't set.
However I don't think that .update()
should be required. I can imagine a scenario where I want to check the view blocks before ever updating. Maybe even to compare it with what the view looks like after updating. This would of course still effectively call an update without doing session migration. I think though that this even ideally wouldn't happen as the view shouldn't require having been updated to read from it. Before that happens I figure building in the guard (recreating it as it was previously) at least fixes requiring a call to .update()
.
I agree. I would definitely prefer not having to manually call update.. I probably ended up here thinking I could get the non-updated linearized core if I didn't update before get, and the way I been working with vanilla hypercores as well.