feat: make api only return json (#42320)

* fix(api): return json for delete + reset-progress

* refactor(api): make .json calls explicit

These .send() calls had objects as arguments. Using .json() makes it
explicit without changing behaviour.

* fix: return json or redirectWithFlash

We should never be getting requests for plain text, but if we do they
should be redirected back to the client.

* fix: prioritize JSON responses

If accepted, respond with JSON.  If not, it's probably a bad request.
This commit is contained in:
Oliver Eyton-Williams
2021-06-05 14:20:40 +02:00
committed by GitHub
parent 989347387f
commit 7b65909522
6 changed files with 20 additions and 29 deletions

View File

@ -286,7 +286,7 @@ function projectCompleted(req, res, next) {
}) })
); );
return Observable.fromPromise(updatePromise).doOnNext(() => { return Observable.fromPromise(updatePromise).doOnNext(() => {
return res.send({ return res.json({
alreadyCompleted, alreadyCompleted,
points: alreadyCompleted ? user.points : user.points + 1, points: alreadyCompleted ? user.points : user.points + 1,
completedDate: completedChallenge.completedDate completedDate: completedChallenge.completedDate
@ -320,7 +320,7 @@ function backendChallengeCompleted(req, res, next) {
}) })
); );
return Observable.fromPromise(updatePromise).doOnNext(() => { return Observable.fromPromise(updatePromise).doOnNext(() => {
return res.send({ return res.json({
alreadyCompleted, alreadyCompleted,
points: alreadyCompleted ? user.points : user.points + 1, points: alreadyCompleted ? user.points : user.points + 1,
completedDate: completedChallenge.completedDate completedDate: completedChallenge.completedDate

View File

@ -20,7 +20,7 @@ export default function donateBoot(app, done) {
if (!user || !body) { if (!user || !body) {
return res return res
.status(500) .status(500)
.send({ error: 'User must be signed in for this request.' }); .json({ error: 'User must be signed in for this request.' });
} }
return Promise.resolve(req) return Promise.resolve(req)
.then( .then(
@ -31,7 +31,7 @@ export default function donateBoot(app, done) {
.then(() => res.status(200).json({ isDonating: true })) .then(() => res.status(200).json({ isDonating: true }))
.catch(err => { .catch(err => {
log(err.message); log(err.message);
return res.status(500).send({ return res.status(500).json({
type: 'danger', type: 'danger',
message: 'Something went wrong.' message: 'Something went wrong.'
}); });

View File

@ -193,7 +193,7 @@ module.exports = function (app) {
pulls === parseInt(pulls, 10) && issues pulls === parseInt(pulls, 10) && issues
? Object.keys(JSON.parse(issues)).length - pulls ? Object.keys(JSON.parse(issues)).length - pulls
: "Can't connect to GitHub"; : "Can't connect to GitHub";
return res.send({ return res.json({
issues: issues, issues: issues,
pulls: pulls pulls: pulls
}); });

View File

@ -185,7 +185,7 @@ function postResetProgress(req, res, next) {
if (err) { if (err) {
return next(err); return next(err);
} }
return res.sendStatus(200); return res.status(200).json({});
} }
); );
} }
@ -199,7 +199,7 @@ function createPostDeleteAccount(app) {
} }
req.logout(); req.logout();
removeCookies(req, res); removeCookies(req, res);
return res.sendStatus(200); return res.status(200).json({});
}); });
}; };
} }

View File

@ -4,20 +4,16 @@ import { getRedirectParams } from '../utils/redirection';
export default function fourOhFour(app) { export default function fourOhFour(app) {
app.all('*', function (req, res) { app.all('*', function (req, res) {
const accept = accepts(req); const accept = accepts(req);
const type = accept.type('html', 'json', 'text'); // prioritise returning json
const type = accept.type('json', 'html', 'text');
const { path } = req; const { path } = req;
const { origin } = getRedirectParams(req); const { origin } = getRedirectParams(req);
if (type === 'html') { if (type === 'json') {
return res.status('404').json({ error: 'path not found' });
} else {
req.flash('danger', `We couldn't find path ${path}`); req.flash('danger', `We couldn't find path ${path}`);
return res.redirectWithFlash(`${origin}/404`); return res.redirectWithFlash(`${origin}/404`);
} }
if (type === 'json') {
return res.status('404').json({ error: 'path not found' });
}
res.setHeader('Content-Type', 'text/plain');
return res.send('404 path not found');
}); });
} }

View File

@ -37,7 +37,8 @@ export default function prodErrorHandler() {
// parse res type // parse res type
const accept = accepts(req); const accept = accepts(req);
const type = accept.type('html', 'json', 'text'); // prioritise returning json
const type = accept.type('json', 'html', 'text');
const redirectTo = handled.redirectTo || `${origin}/`; const redirectTo = handled.redirectTo || `${origin}/`;
const message = const message =
@ -48,22 +49,16 @@ export default function prodErrorHandler() {
console.error(errTemplate(err, req)); console.error(errTemplate(err, req));
} }
if (type === 'html') { if (type === 'json') {
return res.json({
type: handled.type || 'errors',
message
});
} else {
if (typeof req.flash === 'function') { if (typeof req.flash === 'function') {
req.flash(handled.type || 'danger', message); req.flash(handled.type || 'danger', message);
} }
return res.redirectWithFlash(redirectTo); return res.redirectWithFlash(redirectTo);
// json
} else if (type === 'json') {
res.setHeader('Content-Type', 'application/json');
return res.send({
type: handled.type || 'errors',
message
});
// plain text
} else {
res.setHeader('Content-Type', 'text/plain');
return res.send(message);
} }
}; };
} }