Glib-w2-NodeJS #22
Glib-w2-NodeJS #22
Conversation
| export const keys = { | ||
| API_KEY: 'afc0f7157bda937505e237c68802afa5' | ||
| } |
There was a problem hiding this comment.
this is the easy way as requested by the homework description, have you thought of a bit safer more reliable way? maybe you wanna lookup dotenv?
There was a problem hiding this comment.
exposing an API key in your source code is a security risk 🚨
Anyone can steal and misuse it.
The best practice is to hide the API key.
npm install dotenv
Move API key to a .env file (create this in your project root):
and write in it:
API_KEY=afc0f7157bda937505e237c68802afa5
then you can import the dotenv to securely import that api key. Something like this below.
import dotenv from 'dotenv';
dotenv.config();
export const keys = {
API_KEY: process.env.API_KEY
};
| "scripts": { | ||
| "test": "jest", | ||
| "start": "nodemon server.js" | ||
| }, |
| const response = await request | ||
| .post("/weather") | ||
| .send({ cityName }); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(response.body.weatherText).toBe("City not found!"); |
There was a problem hiding this comment.
in such a case don't you think that a 404 Not found as a good fit?
There was a problem hiding this comment.
Your test expects a 200 status code when a city is not found, but logically, it should return 404 (Not Found) instead.
Fix: update it to return res.status(404).json({ error: "City not found!" })
instead of:
res.json({ weatherText: "City not found!" }).
There was a problem hiding this comment.
Use .toMatchObject() Instead of .toBe() for JSON Comparisons.
Update it to: expect(response.body).toMatchObject({ error: "cityName is required" });
| const response = await fetch(`https://api.openweathermap.org/data/2.5/weather?q=${cityName}&appid=${keys.API_KEY}&units=metric`); | ||
| const data = await response.json(); | ||
|
|
||
| if (data.cod !== 200) return res.json({ weatherText: 'City not found!' }); |
There was a problem hiding this comment.
it's a good idea to check for the response.ok and it's not always a check for a 200, not everything comes as okay or not, thus you might wanna change the returned status code to be a good match to the response type. but for basic use and learning purposes only you might just wanna keep it meaningful to the message sent.
have you thought of 404?
There was a problem hiding this comment.
- Check for 404, not 200 in the if condition.
- data.main.temp might be undefined if the API request fails or returns an unexpected response.
Calling Math.floor(undefined) will result in NaN, which is not user-friendly. - You're catching errors but not logging them, which makes debugging harder.
console.error(error); // Logs the error in the console
res.status(500).json({ error: 'Server-side error.' });
No description provided.