DEV Community

loading...

Telescope: Fixing the "small bug" (2/3)

zg3d profile image Devansh Shah ・2 min read

Intro

In the last blog I talked about fixing a small bug in telescope. Well while I was trying to follow the plan. Someone on slack said that previous commit caused this issue and there is also a comment on the issue. Well the fix is simple I can just revert the change which was to edit a docker file.

Issue -> ELASTIC_URL=http://elasticsearch:${ELASTIC_PORT}
Solution -> ELASTIC_URL=http://elasticsearch
Enter fullscreen mode Exit fullscreen mode

Code Review

This was a simple solution but this would not fix the root of the issue in the future if someone changes the environment variable we would have the same problem. So, I created a function in the Elastic.js file to parse the URL.

The First version was the following.

const parseUrl = (url, port) => {
  let result;
  try {
    const urlObj = new URL(url);
    urlObj.port = port;
    if (urlObj.port !== '') {
      result = urlObj.href;
    }
  } catch (e) {
    result = null;
 }
return result;
}
const elasticUrl = parseUrl(ELASTIC_URL, ELASTIC_PORT) || 'http://localhost:9200';
Enter fullscreen mode Exit fullscreen mode

This version changes the port and if it is not an empty port we send the href back or else we can have undefined or null. This version has a couple problems we are returning 2 false values null and undefined and we do not need to have an if statement. I also found out that telescope had many instances of using the legacy NodeJS URL library. So, This function can now globally for all URL with ports coming in from the environment for example redis.js. So, the next step was to make this function global and also update the utilization of NodeJS URL Library throughout the project. I also want to add a bunch of tests for this function.

The difference between the legacy URL library and the current URL library is that the legacy version does not use a class, It instead used an object and functions to parse. So if I wanted to destructure it or parse it I would need to use the parse function like this

// legacy
  const { port, host } = url.parse('http://localhost:3000', true);
// current
  const { port, host } = new URL(''http://localhost:3000'');
Enter fullscreen mode Exit fullscreen mode

The New Plan

To finish this feature I now need to add

  • Make the function global
  • Add Testing
  • Update all instances of legacy URL library use

Discussion (0)

pic
Editor guide