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
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';
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'');
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
Top comments (0)