bugfix: nil dereference in FileApiLite::GetEpoch

[mitjat]

No description provided.

[pro-wh]

what causes this? doesn't it make a real time variable inside and & it? how does it become nil

[mitjat]

how does it become nil

The caching layer can return a nil in some cases. I think #409 might get rid of all those cases, but still, best not to assume too much here.

I wish I had kept better notes, I only remember for sure that I encountered a nil dereference error here in the wild.

[mitjat]

Ah, I've just had it happen again :)

Whenever the internal RPC-calling func returns an error, the KVStore helper replaces the value with nil: https://github.com/oasisprotocol/oasis-indexer/blob/614a82493e881fc2e0584ec969657b64f02e67a0/storage/oasis/nodeapi/file/common.go#L94

Where I encountered it was during shutdown (ctrl+C or k8s SIGTERM), because the RPC-based ConsensusApiLite gets closed mid-request. It should also happen in case of intermittent IO problems etc.

[pro-wh]

KVStore helper replaces the value with nil

but then err != nil and we'd bail before trying to dereference

[pro-wh]

oh wait that link goes to a different function yeah

[pro-wh]

ok I never really looked into why GetFromCacheOrCall required it to be a pointer

[mitjat]

ok I never really looked into why GetFromCacheOrCall required it to be a pointer

It probably technically doesn't, and we could return the default value instead. I used the pointer because functions in Go that return (ValueType, error) seem to prefer ValueType to be a pointer, and a nil signifies to me the optionality of a response a little more clearly. I might well be unknowingly going against a GoLang pointer use convention though :/