Skip to content

Refactored controller errors #76

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
joeljoby02 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Refactored controller errors #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 3 additions & 11 deletions controllers/crud.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { newID, isValidID, db } from '../database/client.js'
import { configureWebAnnoHeadersFor, configureLDHeadersFor, configureLastModifiedHeader } from '../headers.js'
import { configureRerumOptions } from '../versioning.js'
import config from '../config/index.js'
import { _contextid, idNegotiation, generateSlugId, ObjectID, createExpressError, getAgentClaim, parseDocumentID } from './utils.js'
import { _contextid, idNegotiation, generateSlugId, ObjectID, createError, createExpressError, getAgentClaim, parseDocumentID } from './utils.js'

/**
* Create a new Linked Open Data object in RERUM v1.
Expand Down Expand Up @@ -74,11 +74,7 @@ const query = async function (req, res, next) {
const skip = parseInt(req.query.skip ?? 0)
if (Object.keys(props).length === 0) {
//Hey now, don't ask for everything...this can happen by accident. Don't allow it.
let err = {
message: "Detected empty JSON object. You must provide at least one property in the /query request body JSON.",
status: 400
}
next(createExpressError(err))
next(createExpressError(createError(400, "Detected empty JSON object. You must provide at least one property in the /query request body JSON.")))
return
}
try {
Expand Down Expand Up @@ -115,11 +111,7 @@ const id = async function (req, res, next) {
res.json(match)
return
}
let err = {
"message": `No RERUM object with id '${id}'`,
"status": 404
}
next(createExpressError(err))
next(createExpressError(createError(404, `No RERUM object with id '${id}'`)))
} catch (error) {
next(createExpressError(error))
}
Expand Down
39 changes: 14 additions & 25 deletions controllers/delete.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { newID, isValidID, db } from '../database/client.js'
import { isDeleted, isReleased, isGenerator } from '../predicates.js'
import config from '../config/index.js'
import { createExpressError, getAgentClaim, parseDocumentID, getAllVersions, getAllDescendants } from './utils.js'
import { createError, createExpressError, getAgentClaim, parseDocumentID, getAllVersions, getAllDescendants } from './utils.js'

/**
* Mark an object as deleted in the database.
Expand All @@ -23,7 +23,8 @@ import { createExpressError, getAgentClaim, parseDocumentID, getAllVersions, get
* */
const deleteObj = async function(req, res, next) {
let id
let err = { message: `` }
let errMessage = ""
let errStatus = 0
try {
id = req.params["_id"] ?? parseDocumentID(JSON.parse(JSON.stringify(req.body))["@id"]) ?? parseDocumentID(JSON.parse(JSON.stringify(req.body))["id"])
} catch(error){
Expand All @@ -41,25 +42,19 @@ const deleteObj = async function(req, res, next) {
if (null !== originalObject) {
let safe_original = JSON.parse(JSON.stringify(originalObject))
if (isDeleted(safe_original)) {
err = Object.assign(err, {
message: `The object you are trying to delete is already deleted. ${err.message}`,
status: 403
})
errMessage = `The object you are trying to delete is already deleted. ${errMessage}`
errStatus = 403
}
else if (isReleased(safe_original)) {
err = Object.assign(err, {
message: `The object you are trying to delete is released. Fork to make changes. ${err.message}`,
status: 403
})
errMessage = `The object you are trying to delete is released. Fork to make changes. ${errMessage}`
errStatus = 403
}
else if (!isGenerator(safe_original, agentRequestingDelete)) {
err = Object.assign(err, {
message: `You are not the generating agent for this object and so are not authorized to delete it. ${err.message}`,
status: 401
})
errMessage = `You are not the generating agent for this object and so are not authorized to delete it. ${errMessage}`
errStatus = 401
}
if (err.status) {
next(createExpressError(err))
if (errStatus) {
next(createExpressError(createError(errStatus, errMessage)))
return
}
let preserveID = safe_original["@id"]
Expand All @@ -82,9 +77,7 @@ const deleteObj = async function(req, res, next) {
}
if (result.modifiedCount === 0) {
//result didn't error out, the action was not performed. Sometimes, this is a neutral thing. Sometimes it is indicative of an error.
err.message = "The original object was not replaced with the deleted object in the database."
err.status = 500
next(createExpressError(err))
next(createExpressError(createError(500, "The original object was not replaced with the deleted object in the database.")))
return
}
//204 to say it is deleted and there is nothing in the body
Expand All @@ -93,14 +86,10 @@ const deleteObj = async function(req, res, next) {
return
}
//Not sure we can get here, as healHistoryTree might throw and error.
err.message = "The history tree for the object being deleted could not be mended."
err.status = 500
next(createExpressError(err))
next(createExpressError(createError(500, "The history tree for the object being deleted could not be mended.")))
return
}
err.message = "No object with this id could be found in RERUM. Cannot delete."
err.status = 404
next(createExpressError(err))
next(createExpressError(createError(404, "No object with this id could be found in RERUM. Cannot delete.")))
}

/**
Expand Down
37 changes: 14 additions & 23 deletions controllers/overwrite.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { newID, isValidID, db } from '../database/client.js'
import { isDeleted, isReleased, isGenerator } from '../predicates.js'
import { configureWebAnnoHeadersFor } from '../headers.js'
import { _contextid, ObjectID, createExpressError, getAgentClaim, parseDocumentID, idNegotiation } from './utils.js'
import { _contextid, ObjectID, createError, createExpressError, getAgentClaim, parseDocumentID, idNegotiation } from './utils.js'

/**
* Replace some existing object in MongoDB with the JSON object in the request body.
Expand All @@ -18,7 +18,8 @@ import { _contextid, ObjectID, createExpressError, getAgentClaim, parseDocumentI
* Respond RESTfully
* */
const overwrite = async function (req, res, next) {
let err = { message: `` }
let errMessage = ""
let errStatus = 0
res.set("Content-Type", "application/json; charset=utf-8")
let objectReceived = JSON.parse(JSON.stringify(req.body))
let agentRequestingOverwrite = getAgentClaim(req, next)
Expand All @@ -34,28 +35,20 @@ const overwrite = async function (req, res, next) {
return
}
if (null === originalObject) {
err = Object.assign(err, {
message: `No object with this id could be found in RERUM. Cannot overwrite. ${err.message}`,
status: 404
})
errMessage = `No object with this id could be found in RERUM. Cannot overwrite. ${errMessage}`
errStatus = 404
}
else if (isDeleted(originalObject)) {
err = Object.assign(err, {
message: `The object you are trying to overwrite is deleted. ${err.message}`,
status: 403
})
errMessage = `The object you are trying to overwrite is deleted. ${errMessage}`
errStatus = 403
}
else if (isReleased(originalObject)) {
err = Object.assign(err, {
message: `The object you are trying to overwrite is released. Fork with /update to make changes. ${err.message}`,
status: 403
})
errMessage = `The object you are trying to overwrite is released. Fork with /update to make changes. ${errMessage}`
errStatus = 403
}
else if (!isGenerator(originalObject, agentRequestingOverwrite)) {
err = Object.assign(err, {
message: `You are not the generating agent for this object. You cannot overwrite it. Fork with /update to make changes. ${err.message}`,
status: 401
})
errMessage = `You are not the generating agent for this object. You cannot overwrite it. Fork with /update to make changes. ${errMessage}`
errStatus = 401
}
else {
// Optimistic locking check - no expected version is a brutal overwrite
Expand Down Expand Up @@ -105,12 +98,10 @@ const overwrite = async function (req, res, next) {
}
else {
//This is a custom one, the http module will not detect this as a 400 on its own
err = Object.assign(err, {
message: `Object in request body must have the property '@id' or 'id'. ${err.message}`,
status: 400
})
errMessage = `Object in request body must have the property '@id' or 'id'. ${errMessage}`
errStatus = 400
}
next(createExpressError(err))
next(createExpressError(createError(errStatus, errMessage)))
}

export { overwrite }
33 changes: 12 additions & 21 deletions controllers/release.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { newID, isValidID, db } from '../database/client.js'
import { isDeleted, isReleased, isGenerator } from '../predicates.js'
import { configureWebAnnoHeadersFor } from '../headers.js'
import { _contextid, ObjectID, createExpressError, getAgentClaim, parseDocumentID, idNegotiation, generateSlugId, establishReleasesTree, healReleasesTree } from './utils.js'
import { _contextid, ObjectID, createError, createExpressError, getAgentClaim, parseDocumentID, idNegotiation, generateSlugId, establishReleasesTree, healReleasesTree } from './utils.js'

/**
* Public facing servlet to release an existing RERUM object. This will not
Expand All @@ -25,7 +25,8 @@ const release = async function (req, res, next) {
let agentRequestingRelease = getAgentClaim(req, next)
let id = req.params["_id"]
let slug
let err = {"message":""}
let errMessage = ""
let errStatus = 0
let treeHealed = false
if(req.get("Slug")){
let slug_json = await generateSlugId(req.get("Slug"), next)
Expand All @@ -51,25 +52,19 @@ const release = async function (req, res, next) {
let nextReleases = safe_original.__rerum.releases.next

if (isDeleted(safe_original)) {
err = Object.assign(err, {
message: `The object you are trying to release is deleted. ${err.message}`,
status: 403
})
errMessage = `The object you are trying to release is deleted. ${errMessage}`
errStatus = 403
}
if (isReleased(safe_original)) {
err = Object.assign(err, {
message: `The object you are trying to release is already released. ${err.message}`,
status: 403
})
errMessage = `The object you are trying to release is already released. ${errMessage}`
errStatus = 403
}
if (!isGenerator(safe_original, agentRequestingRelease)) {
err = Object.assign(err, {
message: `You are not the generating agent for this object. You cannot release it. ${err.message}`,
status: 401
})
errMessage = `You are not the generating agent for this object. You cannot release it. ${errMessage}`
errStatus = 401
}
if (err.status) {
next(createExpressError(err))
if (errStatus) {
next(createExpressError(createError(errStatus, errMessage)))
return
}
console.log("RELEASE")
Expand Down Expand Up @@ -122,11 +117,7 @@ const release = async function (req, res, next) {
}
else{
//This was a bad request
err = {
message: "You must provide the id of an object to release. Use /release/id-here or release?_id=id-here.",
status: 400
}
next(createExpressError(err))
next(createExpressError(createError(400, "You must provide the id of an object to release. Use /release/id-here or release?_id=id-here.")))
return
}
}
Expand Down
38 changes: 7 additions & 31 deletions controllers/search.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/
import { db } from '../database/client.js'
import { configureLDHeadersFor } from '../headers.js'
import { idNegotiation, createExpressError } from './utils.js'
import { idNegotiation, createError, createExpressError } from './utils.js'

/**
* Merges and deduplicates results from multiple MongoDB Atlas Search index queries.
Expand Down Expand Up @@ -265,11 +265,7 @@ const searchAsWords = async function (req, res, next) {
let searchText = req.body?.searchText ?? req.body
const searchOptions = req.body?.options ?? {}
if (!searchText) {
let err = {
message: "You did not provide text to search for in the search request.",
status: 400
}
next(createExpressError(err))
next(createExpressError(createError(400, "You did not provide text to search for in the search request.")))
return
}
const limit = parseInt(req.query.limit ?? 100)
Expand Down Expand Up @@ -353,11 +349,7 @@ const searchAsPhrase = async function (req, res, next) {
slop: 2
}
if (!searchText) {
let err = {
message: "You did not provide text to search for in the search request.",
status: 400
}
next(createExpressError(err))
next(createExpressError(createError(400, "You did not provide text to search for in the search request.")))
return
}
const limit = parseInt(req.query.limit ?? 100)
Expand Down Expand Up @@ -433,11 +425,7 @@ const searchFuzzily = async function (req, res, next) {
}
}
if (!searchText) {
let err = {
message: "You did not provide text to search for in the search request.",
status: 400
}
next(createExpressError(err))
next(createExpressError(createError(400, "You did not provide text to search for in the search request.")))
return
}
const limit = parseInt(req.query.limit ?? 100)
Expand Down Expand Up @@ -521,20 +509,12 @@ const searchWildly = async function (req, res, next) {
allowAnalyzedField: true
}
if (!searchText) {
let err = {
message: "You did not provide text to search for in the search request.",
status: 400
}
next(createExpressError(err))
next(createExpressError(createError(400, "You did not provide text to search for in the search request.")))
return
}
// Require wildcards in the search text
if (!searchText.includes('*') && !searchText.includes('?')) {
let err = {
message: "Wildcards must be used in wildcard search. Use '*' to match any characters or '?' to match a single character.",
status: 400
}
next(createExpressError(err))
next(createExpressError(createError(400, "Wildcards must be used in wildcard search. Use '*' to match any characters or '?' to match a single character.")))
return
}
const limit = parseInt(req.query.limit ?? 100)
Expand Down Expand Up @@ -625,11 +605,7 @@ const searchAlikes = async function (req, res, next) {
let likeDocument = req.body
// Validate that a document was provided
if (!likeDocument || (typeof likeDocument !== 'object') || Object.keys(likeDocument).length === 0) {
let err = {
message: "You must provide a JSON document in the request body to find similar documents.",
status: 400
}
next(createExpressError(err))
next(createExpressError(createError(400, "You must provide a JSON document in the request body to find similar documents.")))
return
}
const limit = parseInt(req.query.limit ?? 100)
Expand Down
4 changes: 4 additions & 0 deletions controllers/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ const index = function (req, res, next) {
})
}

export function createError(status, message, details = {}) {
return Object.assign(new Error(message), { status, ...details })
}

function createExpressError(err) {
let error = {}
if (err.code) {
Expand Down
Loading