error handling robustness, logger corrections, better playlist-editable checking

This commit is contained in:
Kaushik Narayan R 2025-03-14 20:44:37 -07:00
parent ef3a055c06
commit 14eeb57a0e
4 changed files with 76 additions and 95 deletions

View File

@ -98,7 +98,8 @@ const singleRequest = async <RespDataType>({
message = message.concat(
`${error.response.status} - ${error.response.data?.error?.message}`
);
res?.status(error.response.status).send(error.response.data);
if (res && !res.headersSent)
res.status(error.response.status).send(error.response.data);
logger.warn(message, {
response: {
data: error.response.data,
@ -109,13 +110,14 @@ const singleRequest = async <RespDataType>({
} else if (error.request) {
// Request sent, but no response received
message = message.concat("No response");
res?.status(504).send({ message });
if (res && !res.headersSent) res.status(504).send({ message });
logger.error(message, { error });
return { error, message };
} else {
// Something happened in setting up the request that triggered an Error
message = message.concat("Request failed");
res?.status(500).send({ message: "Internal Server Error" });
if (res && !res.headersSent)
res.status(500).send({ message: "Internal Server Error" });
logger.error(message, { error });
return { error, message };
}
@ -168,9 +170,7 @@ const getCurrentUsersPlaylistsNextPage: (
});
};
interface GetPlaylistDetailsFirstPageArgs
extends Omit<EndpointHandlerWithResArgs, "res"> {
res?: Res;
interface GetPlaylistDetailsFirstPageArgs extends EndpointHandlerWithResArgs {
initialFields: string;
playlistID: string;
}
@ -257,9 +257,7 @@ const removePlaylistItems: (
// non-endpoints, i.e. convenience wrappers
// ---------
interface CheckPlaylistEditableArgs
extends Omit<EndpointHandlerWithResArgs, "res"> {
res?: Res;
interface CheckPlaylistEditableArgs extends EndpointHandlerWithResArgs {
playlistID: string;
userID: string;
}
@ -276,14 +274,13 @@ const checkPlaylistEditable: (
playlistID,
userID,
}) => {
let checkFields = ["collaborative", "owner(id)"];
let args: GetPlaylistDetailsFirstPageArgs = {
let checkFields = ["collaborative", "owner(id)", "name"];
const { resp, error, message } = await getPlaylistDetailsFirstPage({
res,
authHeaders,
initialFields: checkFields.join(),
playlistID,
};
if (res) args.res = res;
const { resp, error, message } = await getPlaylistDetailsFirstPage(args);
});
if (!resp) return { status: false, error, message };
// https://web.archive.org/web/20241226081630/https://developer.spotify.com/documentation/web-api/concepts/playlists#:~:text=A%20playlist%20can%20also%20be%20made%20collaborative

View File

@ -79,10 +79,10 @@ const callback: RequestHandler = async (req, res) => {
Authorization: `Bearer ${req.session.accessToken}`,
};
} else {
logger.error("login failed", { statusCode: tokenResponse.status });
res
.status(tokenResponse.status)
.send({ message: "Error: Login failed" });
logger.error("login failed", { statusCode: tokenResponse.status });
return null;
}

View File

@ -154,9 +154,7 @@ const updateUser: RequestHandler = async (req, res) => {
});
if (delNum !== deleted.length) {
res.status(500).send({ message: "Internal Server Error" });
logger.error("Could not remove all old playlists", {
error: new Error("Playlists.destroy failed?"),
});
logger.error("Could not remove all old playlists");
return null;
}
}
@ -170,9 +168,7 @@ const updateUser: RequestHandler = async (req, res) => {
);
if (addPls.length !== added.length) {
res.status(500).send({ message: "Internal Server Error" });
logger.error("Could not add all new playlists", {
error: new Error("Playlists.bulkCreate failed?"),
});
logger.error("Could not add all new playlists");
return null;
}
}
@ -189,9 +185,7 @@ const updateUser: RequestHandler = async (req, res) => {
});
} catch (error) {
res.status(500).send({ message: "Internal Server Error" });
logger.error("Could not update playlist names", {
error: new Error("Playlists.update failed?"),
});
logger.error("Could not update playlist names");
return null;
}
@ -272,13 +266,13 @@ const createLink: RequestHandler = async (req, res) => {
fromPl = parseSpotifyLink(req.body.from);
toPl = parseSpotifyLink(req.body.to);
if (fromPl.type !== "playlist" || toPl.type !== "playlist") {
res.status(400).send({ message: "Link is not a playlist" });
logger.info("non-playlist link provided", { from: fromPl, to: toPl });
res.status(400).send({ message: "Links must be playlist links!" });
logger.debug("non-playlist link provided");
return null;
}
} catch (error) {
res.status(400).send({ message: "Could not parse link" });
logger.warn("parseSpotifyLink", { error });
logger.info("parseSpotifyLink", { error });
return null;
}
@ -291,8 +285,8 @@ const createLink: RequestHandler = async (req, res) => {
// if playlists are unknown
if (![fromPl, toPl].every((pl) => playlistIDs.includes(pl.id))) {
res.status(404).send({ message: "Playlists out of sync." });
logger.warn("unknown playlists, resync");
res.status(404).send({ message: "Unknown playlists, resync first." });
logger.debug("unknown playlists, resync");
return null;
}
@ -304,7 +298,7 @@ const createLink: RequestHandler = async (req, res) => {
});
if (existingLink) {
res.status(409).send({ message: "Link already exists!" });
logger.info("link already exists");
logger.debug("link already exists");
return null;
}
@ -322,8 +316,8 @@ const createLink: RequestHandler = async (req, res) => {
if (newGraph.detectCycle()) {
res
.status(400)
.send({ message: "Proposed link cannot cause a cycle in the graph" });
logger.warn("potential cycle detected");
.send({ message: "The link cannot cause a cycle in the graph." });
logger.debug("potential cycle detected");
return null;
}
@ -334,9 +328,7 @@ const createLink: RequestHandler = async (req, res) => {
});
if (!newLink) {
res.status(500).send({ message: "Internal Server Error" });
logger.error("Could not create link", {
error: new Error("Links.create failed?"),
});
logger.error("Could not create link");
return null;
}
@ -364,13 +356,13 @@ const removeLink: RequestHandler = async (req, res) => {
fromPl = parseSpotifyLink(req.body.from);
toPl = parseSpotifyLink(req.body.to);
if (fromPl.type !== "playlist" || toPl.type !== "playlist") {
res.status(400).send({ message: "Link is not a playlist" });
logger.info("non-playlist link provided", { from: fromPl, to: toPl });
res.status(400).send({ message: "Links must be playlist links!" });
logger.debug("non-playlist link provided");
return null;
}
} catch (error) {
res.status(400).send({ message: "Could not parse link" });
logger.warn("parseSpotifyLink", { error });
logger.info("parseSpotifyLink", { error });
return null;
}
@ -382,7 +374,7 @@ const removeLink: RequestHandler = async (req, res) => {
});
if (!existingLink) {
res.status(409).send({ message: "Link does not exist!" });
logger.warn("link does not exist");
logger.debug("link does not exist");
return null;
}
@ -393,9 +385,7 @@ const removeLink: RequestHandler = async (req, res) => {
});
if (!removedLink) {
res.status(500).send({ message: "Internal Server Error" });
logger.error("Could not remove link", {
error: new Error("Links.destroy failed?"),
});
logger.error("Could not remove link");
return null;
}
@ -560,12 +550,12 @@ const populateSingleLink: RequestHandler = async (req, res) => {
toPl = parseSpotifyLink(link.to);
if (fromPl.type !== "playlist" || toPl.type !== "playlist") {
res.status(400).send({ message: "Link is not a playlist" });
logger.info("non-playlist link provided", link);
logger.debug("non-playlist link provided", { link });
return null;
}
} catch (error) {
res.status(400).send({ message: "Could not parse link" });
logger.warn("parseSpotifyLink", { error });
logger.info("parseSpotifyLink", { error });
return null;
}
@ -577,21 +567,21 @@ const populateSingleLink: RequestHandler = async (req, res) => {
});
if (!existingLink) {
res.status(409).send({ message: "Link does not exist!" });
logger.warn("link does not exist", { link });
logger.debug("link does not exist", { link });
return null;
}
if (
!(
await checkPlaylistEditable({
authHeaders,
res,
playlistID: fromPl.id,
userID: uID,
})
).status
)
const editableResp = await checkPlaylistEditable({
res,
authHeaders,
playlistID: fromPl.id,
userID: uID,
});
if (!editableResp.status) {
res.status(403).send({ message: editableResp.message });
logger.debug(editableResp.message, { editableResp });
return null;
}
const fromTracks = await _getPlaylistTracks({
res,
@ -659,12 +649,12 @@ const populateChain: RequestHandler = async (req, res) => {
rootPl = parseSpotifyLink(root);
if (rootPl.type !== "playlist") {
res.status(400).send({ message: "Link is not a playlist" });
logger.info("non-playlist link provided", root);
logger.debug("non-playlist link provided");
return null;
}
} catch (error) {
res.status(400).send({ message: "Could not parse link" });
logger.warn("parseSpotifyLink", { error });
logger.info("parseSpotifyLink", { error });
return null;
}
@ -691,14 +681,26 @@ const populateChain: RequestHandler = async (req, res) => {
const editableStatuses = await Promise.all(
affectedPlaylists.map((pl) => {
return checkPlaylistEditable({
res,
authHeaders,
playlistID: pl,
userID: uID,
});
})
);
if (editableStatuses.some((statusObj) => statusObj.status === false))
if (res.headersSent) return null; // error, resp sent and logged in singleRequest
// else, respond with the non-editable playlists
const nonEditablePlaylists = editableStatuses.filter(
(statusObj) => statusObj.status === false
);
if (nonEditablePlaylists.length > 0) {
let message =
"Cannot edit one or more playlists: " +
nonEditablePlaylists.map((pl) => pl.error?.playlistName).join(", ");
res.status(403).send({ message });
logger.debug(message, { nonEditablePlaylists });
return null;
}
const affectedPlaylistsTracks = await Promise.all(
affectedPlaylists.map((pl) => {
@ -830,12 +832,12 @@ const pruneSingleLink: RequestHandler = async (req, res) => {
toPl = parseSpotifyLink(link.to);
if (fromPl.type !== "playlist" || toPl.type !== "playlist") {
res.status(400).send({ message: "Link is not a playlist" });
logger.info("non-playlist link provided", link);
logger.debug("non-playlist link provided");
return null;
}
} catch (error: any) {
res.status(400).send({ message: error.message });
logger.warn("parseSpotifyLink", { error });
logger.info("parseSpotifyLink", { error });
return null;
}
@ -851,17 +853,17 @@ const pruneSingleLink: RequestHandler = async (req, res) => {
return null;
}
if (
!(
await checkPlaylistEditable({
authHeaders,
res,
playlistID: toPl.id,
userID: uID,
})
).status
)
const editableResp = await checkPlaylistEditable({
res,
authHeaders,
playlistID: toPl.id,
userID: uID,
});
if (!editableResp.status) {
res.status(403).send({ message: editableResp.message });
logger.debug(editableResp.message, { editableResp });
return null;
}
const fromTracks = await _getPlaylistTracks({
res,

View File

@ -2,36 +2,22 @@ import path from "path";
import { createLogger, transports, config, format, type Logger } from "winston";
const { combine, timestamp, printf, errors } = format;
const { combine, timestamp, printf } = format;
const metaFormat = (meta: object) => {
const disallowedKeySets = [{ type: Error, keys: ["stack"] }];
if (Object.keys(meta).length > 0)
return (
"\n" +
JSON.stringify(
meta,
Object.getOwnPropertyNames(meta).filter((key) => {
for (const pair of disallowedKeySets) {
if (meta instanceof pair.type) {
return !pair.keys.includes(key);
}
}
return true;
}),
"\t"
)
);
return "\n" + JSON.stringify(meta, null, "\t");
return "";
};
const logFormat = printf(({ level, message, label, timestamp, ...meta }) => {
const logFormat = printf(({ level, message, timestamp, stack, ...meta }) => {
const errorObj: Error = meta["error"] as Error;
if (errorObj) {
const stackStr = errorObj["stack"];
return (
`${timestamp} [${level.toUpperCase()}]: ${message}` + // line 1
`${metaFormat(errorObj)}\n` + // metadata
`${errorObj["stack"] ?? ""}` // stack trace if any
`${stackStr}` // stack trace if any
);
}
return `${timestamp} [${level.toUpperCase()}]: ${message}${metaFormat(meta)}`;
@ -39,11 +25,7 @@ const logFormat = printf(({ level, message, label, timestamp, ...meta }) => {
const winstonLogger: Logger = createLogger({
levels: config.npm.levels,
format: combine(
errors({ stack: true }),
timestamp({ format: "YYYY-MM-DD HH:mm:ss" }),
logFormat
),
format: combine(timestamp({ format: "YYYY-MM-DD HH:mm:ss" }), logFormat),
transports: [
new transports.Console({ level: "info" }),
new transports.File({
@ -58,7 +40,7 @@ const winstonLogger: Logger = createLogger({
],
});
winstonLogger.on("error", (error) =>
winstonLogger.error("Error inside logger", { error })
console.error("Error inside logger", error)
);
winstonLogger.exceptions.handle(
new transports.File({