ssbc / ssb-ref

check if a string is a valid ssb-reference
MIT License
14 stars 10 forks source link

add basic fix to stop errors in multiserver parsing #27

Closed mixmix closed 5 years ago

mixmix commented 5 years ago

Problem : fresh installs or re-indexes currently explode, because address[1] is undefined. This brings the whole app to a halt and the interface is completely un-useable because the indexing never finishes.

I don't know if this is a good solutions but it's an interim solution which lets my interface load. I'm going to publish this immediately because the current HEAD is so breaking.

mixmix commented 5 years ago

published as v2.13.6

@dominictarr can you loop back and check if there's any better way you want this logic adjusted? Either implement it and publish, OR tell me what to do clearly.

dominictarr commented 5 years ago

well, this could be a reasonably solution, but I'd need to know what inputs it is failing on. I suggest removing this fix, and adding logging to see what caused it, then turn that a test case. just merging the fix means we don't know what the problem really was, and those two lines don't have test coverage, so someone might remove them while refactoring because the tests still pass.

mixmix commented 5 years ago

I did put logging in. The code here assumes an array of arrays where each has 2 entries. For some inputs, multiserver-address inner arrays which had only one entry, so going address[1].name explodes.

I needed a fix immediately and didn't want to role back to where-ever. I already burnt a bunch of time with ssb-ref this week which has been a bit frustrating (this happens, I'm just being up front about what else is influencing)

Agree this needs a better fix

Which module is dropping the ball. multiserver-address, or ssb-ref? The failing test can be found by deleting your backlinks index, taking out my code and watching it try to re-index

dominictarr commented 5 years ago

@mixmix you said:

Either implement it and publish, OR tell me what to do clearly.

then I said, we need a test case. then you said, I don't want to do that?

Well, I can't make you do that, but given that lots of things, maintained by too-busy people such as your self, depend on things like ssb-ref, so we need tests. We'd never have the level of stability that we do have without tests. As a policy, we can't go making changes to back end stuff without test coverage.