refactor: one-at-a-time spotify api calls, some other modifications and logging reordering

boilerplates/controller.js

@@ -10,8 +10,9 @@ const __controller_func = async (req, res) => {
 	try {
 
 	} catch (error) {
+		res.sendStatus(500);
 		logger.error('__controller_func', { error });
-		return res.sendStatus(500);
+		return;
 	}
 }
 

controllers/apiCall.js

@@ -0,0 +1,53 @@
+
+const logger = require("../utils/logger")(module);
+
+const typedefs = require("../typedefs");
+
+const { axiosInstance } = require("../utils/axios");
+
+/**
+ * Spotify API - one-off request handler
+ * @param {typedefs.Req} req needed for auto-placing headers from middleware
+ * @param {typedefs.Res} res handle failure responses here itself
+ * @param {typedefs.AxiosMethod} method HTTP method
+ * @param {string} path request path
+ * @param {typedefs.AxiosRequestConfig} config request params, headers, etc.
+ * @param {any} data request body
+ * @param {boolean} inlineData true if data is to be placed inside config
+ * @returns {Promise<{ success: boolean, resp?: any }>}
+ */
+const singleRequest = async (req, res, method, path, config = {}, data = null, inlineData = false) => {
+	let resp;
+	config.headers = { ...config.headers, ...req.sessHeaders };
+	try {
+		if (!data || (data && inlineData)) {
+			if (data)
+				config.data = data ?? null;
+			resp = await axiosInstance[method.toLowerCase()](path, config);
+		} else {
+			resp = await axiosInstance[method.toLowerCase()](path, data, config);
+		}
+
+		if (resp.status >= 400 && resp.status < 500) {
+			res.status(resp.status).send(resp.data);
+			logger.debug("4XX Response", { resp });
+			return { success: false };
+		}
+		else if (resp.status >= 500) {
+			res.sendStatus(resp.status);
+			logger.warn("5XX Response", { resp });
+			return { success: false };
+		}
+
+		logger.debug("Response received.");
+		return { success: true, resp };
+	} catch (error) {
+		res.sendStatus(500);
+		logger.error("Request threw an error.", { error });
+		return { success: false };
+	}
+}
+
+module.exports = {
+	singleRequest,
+}

controllers/operations.js

@@ -6,6 +6,7 @@ const myGraph = require("../utils/graph");
 const { parseSpotifyLink } = require("../utils/spotifyURITransformer");
 
 const { Op } = require("sequelize");
+const { singleRequest } = require("./apiCall");
 /** @type {typedefs.Model} */
 const Playlists = require("../models").playlists;
 /** @type {typedefs.Model} */
@@ -344,11 +345,14 @@ const populateMissingInLink = async (req, res) => {
 			fromPl = parseSpotifyLink(req.body["from"]);
 			toPl = parseSpotifyLink(req.body["to"]);
 			if (fromPl.type !== "playlist" || toPl.type !== "playlist") {
-				return res.status(400).send({ message: "Link is not a playlist" });
+				res.status(400).send({ message: "Link is not a playlist" });
+				logger.warn("non-playlist link provided", { from: fromPl, to: toPl });
+				return;
 			}
 		} catch (error) {
+			res.status(400).send({ message: "Invalid Spotify playlist link" });
 			logger.error("parseSpotifyLink", { error });
-			return res.status(400).send({ message: "Invalid Spotify playlist link" });
+			return;
 		}
 
 		// check if exists
@@ -362,32 +366,33 @@ const populateMissingInLink = async (req, res) => {
 			}
 		});
 		if (!existingLink) {
+			res.sendStatus(409);
 			logger.error("link does not exist");
-			return res.sendStatus(409);
+			return;
 		}
 
 		let checkFields = ["collaborative", "owner(id)"];
-		const checkFromData = await axiosInstance.get(
+
+		const checkResp = await singleRequest(req, res,
+			"GET",
 			`/playlists/${fromPl.id}/`,
 			{
 				params: {
 					fields: checkFields.join()
 				},
-				headers: req.sessHeaders
-			}
-		);
-		if (checkFromData.status >= 400 && checkFromData.status < 500)
-			return res.status(checkFromData.status).send(checkFromData.data);
-		else if (checkFromData.status >= 500)
-			return res.sendStatus(checkFromData.status);
+			});
+		if (!checkResp.success) return;
+
+		const checkFromData = checkResp.resp.data;
 
 		// editable = collaborative || user is owner
-		if (checkFromData.data.collaborative !== true &&
-			checkFromData.data.owner.id !== uID) {
-			logger.error("user cannot edit target playlist");
-			return res.status(403).send({
+		if (checkFromData.collaborative !== true &&
+			checkFromData.owner.id !== uID) {
+			res.status(403).send({
 				message: "You cannot edit this playlist, you must be owner/playlist must be collaborative"
 			});
+			logger.error("user cannot edit target playlist");
+			return;
 		}
 
 		let initialFields = ["tracks(next,items(is_local,track(uri)))"];
@@ -508,25 +513,25 @@ const populateMissingInLink = async (req, res) => {
 
 		const logNum = toTrackURIs.length;
 
-		// add in batches of 100
+		// append to end in batches of 100
 		while (toTrackURIs.length) {
 			const nextBatch = toTrackURIs.splice(0, 100);
-			const addResponse = await axiosInstance.post(
+			const addResponse = await singleRequest(req, res,
+				"POST",
 				`/playlists/${fromPl.id}/tracks`,
-				{ uris: nextBatch },
-				{ headers: req.sessHeaders }
+				{},
+				{ uris: nextBatch }, false
 			);
-			if (addResponse.status >= 400 && addResponse.status < 500)
-				return res.status(addResponse.status).send(addResponse.data);
-			else if (addResponse.status >= 500)
-				return res.sendStatus(addResponse.status);
+			if (!addResponse.success) return;
 		}
 
+		res.status(200).send({ message: `Added ${logNum} tracks.` });
 		logger.info(`Backfilled ${logNum} tracks`);
-		return res.sendStatus(200);
+		return;
 	} catch (error) {
+		res.sendStatus(500);
 		logger.error('populateMissingInLink', { error });
-		return res.sendStatus(500);
+		return;
 	}
 }
 
@@ -556,11 +561,14 @@ const pruneExcessInLink = async (req, res) => {
 			fromPl = parseSpotifyLink(req.body["from"]);
 			toPl = parseSpotifyLink(req.body["to"]);
 			if (fromPl.type !== "playlist" || toPl.type !== "playlist") {
-				return res.status(400).send({ message: "Link is not a playlist" });
+				res.status(400).send({ message: "Link is not a playlist" });
+				logger.warn("non-playlist link provided", { from: fromPl, to: toPl });
+				return
 			}
 		} catch (error) {
+			res.status(400).send({ message: "Invalid Spotify playlist link" });
 			logger.error("parseSpotifyLink", { error });
-			return res.status(400).send({ message: "Invalid Spotify playlist link" });
+			return;
 		}
 
 		// check if exists
@@ -574,32 +582,33 @@ const pruneExcessInLink = async (req, res) => {
 			}
 		});
 		if (!existingLink) {
+			res.sendStatus(409);
 			logger.error("link does not exist");
-			return res.sendStatus(409);
+			return
 		}
 
 		let checkFields = ["collaborative", "owner(id)"];
-		const checkToData = await axiosInstance.get(
+
+		const checkToResp = await singleRequest(req, res,
+			"GET",
 			`/playlists/${toPl.id}/`,
 			{
 				params: {
 					fields: checkFields.join()
 				},
-				headers: req.sessHeaders
-			}
-		);
-		if (checkToData.status >= 400 && checkToData.status < 500)
-			return res.status(checkToData.status).send(checkToData.data);
-		else if (checkToData.status >= 500)
-			return res.sendStatus(checkToData.status);
+			});
+
+		if (!checkToResp.success) return;
+		const checkToData = checkToResp.resp.data;
 
 		// editable = collaborative || user is owner
-		if (checkToData.data.collaborative !== true &&
-			checkToData.data.owner.id !== uID) {
-			logger.error("user cannot edit target playlist");
-			return res.status(403).send({
+		if (checkToData.collaborative !== true &&
+			checkToData.owner.id !== uID) {
+			res.status(403).send({
 				message: "You cannot edit this playlist, you must be owner/playlist must be collaborative"
 			});
+			logger.error("user cannot edit target playlist");
+			return;
 		}
 
 		let initialFields = ["snapshot_id", "tracks(next,items(is_local,track(uri)))"];
@@ -717,39 +726,37 @@ const pruneExcessInLink = async (req, res) => {
 		const fromTrackURIs = fromPlaylist.tracks.map(track => track.uri);
 		let indexedToTrackURIs = toPlaylist.tracks;
 
-		// forEach doesn't execute in given order, not sure what it uses to order
 		indexedToTrackURIs.forEach((track, index) => {
 			track.position = index;
 		});
 
-		let indexes = indexedToTrackURIs.filter(track => !fromTrackURIs.includes(track.uri)); // only ones missing from the 'from' playlist
+		let indexes = indexedToTrackURIs.filter(track => !fromTrackURIs.includes(track.uri)); // only those missing from the 'from' playlist
 		indexes = indexes.map(track => track.position); // get track positions
 
 		const logNum = indexes.length;
 
-		// remove in batches of 100 (from reverse, to preserve positions)
+		// remove in batches of 100 (from reverse, to preserve positions while modifying)
 		let currentSnapshot = toPlaylist.snapshot_id;
 		while (indexes.length) {
 			const nextBatch = indexes.splice(Math.max(indexes.length - 100, 0), 100);
-			const delResponse = await axiosInstance.delete(
+			const delResponse = await singleRequest(req, res,
+				"DELETE",
 				`/playlists/${toPl.id}/tracks`,
-				{
-					headers: req.sessHeaders,
-					data: { positions: nextBatch, snapshot_id: currentSnapshot }, // delete method doesn't have separate arg for body 
-				}
-			);
-			if (delResponse.status >= 400 && delResponse.status < 500)
-				return res.status(delResponse.status).send(delResponse.data);
-			else if (delResponse.status >= 500)
-				return res.sendStatus(delResponse.status);
-			currentSnapshot = delResponse.data.snapshot_id;
+				{},
+				// axios delete method doesn't have separate arg for body so hv to put it in config
+				{ positions: nextBatch, snapshot_id: currentSnapshot }, true
+			)
+			if (!delResponse.success) return;
+			currentSnapshot = delResponse.resp.data.snapshot_id;
 		}
 
+		res.status(200).send({ message: `Removed ${logNum} tracks.` });
 		logger.info(`Pruned ${logNum} tracks`);
-		return res.sendStatus(200);
+		return;
 	} catch (error) {
+		res.sendStatus(500);
 		logger.error('pruneExcessInLink', { error });
-		return res.sendStatus(500);
+		return;
 	}
 }
 

controllers/playlists.js

@@ -71,8 +71,9 @@ const getUserPlaylists = async (req, res) => {
 
 		delete userPlaylists.next;
 
+		res.status(200).send(userPlaylists);
 		logger.debug("Fetched user's playlists", { num: userPlaylists.total });
-		return res.status(200).send(userPlaylists);
+		return;
 	} catch (error) {
 		logger.error('getUserPlaylists', { error });
 		return res.sendStatus(500);
@@ -97,11 +98,14 @@ const getPlaylistDetails = async (req, res) => {
 		try {
 			uri = parseSpotifyLink(req.query.playlist_link)
 			if (uri.type !== "playlist") {
-				return res.status(400).send({ message: "Link is not a playlist" });
+				res.status(400).send({ message: "Link is not a playlist" });
+				logger.warn("non-playlist link provided", { uri });
+				return;
 			}
 		} catch (error) {
+			res.status(400).send({ message: "Invalid Spotify playlist link" });
 			logger.error("parseSpotifyLink", { error });
-			return res.status(400).send({ message: "Invalid Spotify playlist link" });
+			return;
 		}
 
 		const response = await axiosInstance.get(
@@ -178,11 +182,13 @@ const getPlaylistDetails = async (req, res) => {
 
 		delete playlist.next;
 
+		res.status(200).send(playlist);
 		logger.info("Fetched playlist tracks", { num: playlist.tracks.length });
-		return res.status(200).send(playlist);
+		return;
 	} catch (error) {
+		res.sendStatus(500);
 		logger.error('getPlaylistDetails', { error });
-		return res.sendStatus(500);
+		return;
 	}
 }
 

middleware/authCheck.js

@@ -18,17 +18,19 @@ const isAuthenticated = (req, res, next) => {
 	} else {
 		const delSession = req.session.destroy((err) => {
 			if (err) {
+				res.sendStatus(500);
 				logger.error("Error while destroying session.", { err });
-				return res.sendStatus(500);
+				return;
 			} else {
-				logger.info("Session invalid, destroyed.", { sessionID: delSession.id });
 				res.clearCookie(sessionName);
-				return res.sendStatus(401);
+				res.sendStatus(401);
+				logger.debug("Session invalid, destroyed.", { sessionID: delSession.id });
+				return;
 			}
 		});
 	}
 }
 
 module.exports = {
-	isAuthenticated
+	isAuthenticated,
 }

typedefs.js

@@ -5,6 +5,9 @@
  * @typedef {import('express').Response} Res
  * @typedef {import('express').NextFunction} Next
  * 
+ * @typedef {import('axios').Method} AxiosMethod
+ * @typedef {import('axios').AxiosRequestConfig} AxiosRequestConfig
+ * 
  * @typedef {import("sequelize").Sequelize} Sequelize
  * @typedef {import("sequelize").Model} Model
  * @typedef {import("sequelize").QueryInterface} QueryInterface

utils/axios.js

@@ -32,40 +32,9 @@ axiosInstance.interceptors.request.use(config => {
 axiosInstance.interceptors.response.use(
 	(response) => response,
 	(error) => {
-		if (error.response && error.response.status === 429) {
-			// Rate limiting
-			logger.warn("Spotify API: Too many requests");
+		logger.warn("AxiosError", { req: error.config });
+		if (error.response)
 			return error.response;
-		}
-		else if (error.response) {
-			// Server has responded
-			logger.error(
-				"Spotify API: Error", {
-				response: {
-					status: error.response.status,
-					statusText: error.response.statusText,
-					data: error.response.data
-				}
-			});
-			return error.response;
-		} else if (error.request) {
-			// The request was made but no response was received
-			logger.error(
-				"Spotify API: No response", {
-				request: {
-					url: error.request?.url,
-					params: error.request?.params,
-				}
-			});
-		} else {
-			// Something happened in setting up the request that triggered an Error
-			logger.error(
-				"Spotify API: Request error", {
-				error: {
-					message: error.message,
-				}
-			});
-		}
 		return Promise.reject(error);
 	}
 );