Skip to content
This repository has been archived by the owner on Jun 21, 2020. It is now read-only.

Handle AnnounceLocalStateAction in worker/controller correctly. #15

Open
Isan-Rivkin opened this issue Nov 18, 2018 · 0 comments
Open
Assignees
Labels
TODO Some functionality that should be added

Comments

@Isan-Rivkin
Copy link
Contributor

When providing content the error handling is very wrong and misleading.
It might look like providing content didn't work while it actually works.
Copying here all the code on purpose since the code might change and there's no point in links.
See the TODO in the code to get better context.

const constants = require('../../../../common/constants');
const EngCid = require('../../../../common/EngCID');
/**
 * This Action announces to the network about it's local state
 * This should be called once the node is synched with the network and has all the deltas and contracts.
 * //TODO:: add flag to check if and only if NODE_IS_FULLY_SYNCED then allow otherwise dismiss the request
 * */
class AnnounceLocalStateAction{
  constructor(controller){
    this._controller = controller;
  }
  execute(params){
    let useCache = params.cache;
    let onResponse = params.onResponse;
    let isEngCid = params.isEngCid;

    if(useCache){
      this._controller.cache().getAllTips((err,tipsList)=>{
        //TODO:: implement cache logic
        //TODO:: if cache empty still query core since maybe it was deleted or first time
      });
    }else{
      this._controller.execCmd(constants.NODE_NOTIFICATIONS.GET_ALL_ADDRS,{
        useCache : useCache,
        onResponse : (err,allAddrsResponse)=>{
          /**
           * do the announcement
           * */
          let parsedEngCids = allAddrsResponse.addresses.map(addr=>{
            let ecid = EngCid.createFromKeccack256(addr);
            if(ecid){
              return ecid;
            }else{
              console.log('[-] err converting bytearry->hex->EngCid !');
            }
          }).filter(ecid=>{return (ecid !== undefined && ecid !== null);});
          isEngCid = true;
          this._controller.provider().provideContentsBatch(parsedEngCids, isEngCid,(err, failedCids)=>{
            if(err){
              //TODO:: this is completley incorrect.
              //TODO:: it shows like there was some total error, but there will be errrors in the case where 1 peer logged out
              //TODO:: it will try to reconnect to him in the DHT and will throw an error
              //TODO:: this is ok accestable behaviour.
              //TODO:: the log below is missleading it says that there was a general error but that's no true, everything still works/
              //TODO:: Bottom line im not processing the errors in the correct way.
              console.log('[-] err %s couldnt provide at all. failed cids %s ', err, failedCids.length);
              onResponse(err,parsedEngCids);
            }else{
              console.log('[+] success providing cids. there are failed %s cids  ', failedCids.length);
              onResponse(null,parsedEngCids);
            }
          });
        }
      });
    }
  }
}
module.exports = AnnounceLocalStateAction;
@Isan-Rivkin Isan-Rivkin added the TODO Some functionality that should be added label Nov 18, 2018
@Isan-Rivkin Isan-Rivkin self-assigned this Nov 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
TODO Some functionality that should be added
Projects
None yet
Development

No branches or pull requests

1 participant