return grpc error when gateway not found

[andymck]

This fixes an issue whereby the find_gateway follower API was not handling a gateway not found.

When this scenario is encountered, a GRPC error will now be returned.

Ideally we would not return a GRPC level error and instead return an application level error as we do with the gateway service but we can extend to that in a follow up if desired

[andymck]

Why not return an application level error since it’s the right thing to do?

If we need it out to fix an ongoing issue this is the quickest thing to do. We will have to refactor on the client side to handle the new response types. I dont have cycles to do this right now...but if someone else wants to pick it up...it is the right thing to do

[jeffgrunewald]

Looks like blockchain_ledger_gateway_v2:gain/1 can also return undefined; should we also handle that and return an empty binary in that case as well?

[andymck]

Looks like blockchain_ledger_gateway_v2:gain/1 can also return undefined; should we also handle that and return an empty binary in that case as well?

why would we want to return an empty binary ? i dont actually get why we do it with location in the first place

[jeffgrunewald]

Looks like blockchain_ledger_gateway_v2:gain/1 can also return undefined; should we also handle that and return an empty binary in that case as well?

why would we want to return an empty binary ? i dont actually get why we do it with location in the first place

Maybe not an empty binary, but whatever is an appropriate “None” value when a partial response is still preferred to an error. I agree before when we only returned the pubkey we requested and the block height of the response and everything else was empty should have instead been an application error, but now that we’re packing in more data into the response we need to return something for an undefined field value or else fail the entire response and return an error, no? Will the proto response struct automatically convert an undefined field into a None?

[jeffgrunewald]

@jeffgrunewald I'd prefer a proper error is returned

I didn’t know how critical it was to get this “working” vs get it correct immediately. I’ve created a task on the board for this since it’s not anyone’s first priority at the moment so as not to lose track of the issue: https://github.com/helium/blockchain-node/issues/182

[andymck]

Looks like blockchain_ledger_gateway_v2:gain/1 can also return undefined; should we also handle that and return an empty binary in that case as well?

why would we want to return an empty binary ? i dont actually get why we do it with location in the first place

Maybe not an empty binary, but whatever is an appropriate “None” value when a partial response is still preferred to an error. I agree before when we only returned the pubkey we requested and the block height of the response and everything else was empty should have instead been an application error, but now that we’re packing in more data into the response we need to return something for an undefined field value or else fail the entire response and return an error, no? Will the proto response struct automatically convert an undefined field into a None?

In the case of the gain field that is an integer value and as the server side is erlang based records as the source ( which are then converted to protos ), these will default to zero for any unset value

[andymck]

This will obviously need updated to account for any changes to the associated proto PR but getting https://github.com/helium/oracles/pull/115/ merged first would also be recommended and apply the handling of the error responses there