Skip to content

Commit

Permalink
fix: improve API error-handling (#1301)
Browse files Browse the repository at this point in the history
Unleash is an API and it would simplyfy a lot of the specific
errors could carry the expected HTTP status code for this error.
This would eliminate the need for a gigantic switch/case in the
handle-errors function.
  • Loading branch information
ivarconr authored Jan 26, 2022
1 parent cd5f3bb commit a50d0e2
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/lib/error/bad-data-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ class BadDataError extends Error {
};
}
}

export default BadDataError;
module.exports = BadDataError;
26 changes: 26 additions & 0 deletions src/lib/error/base-error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
class BaseError extends Error {
readonly statusCode: number;

constructor(message: string, statusCode: number, name: string) {
super();

this.name = name;
this.message = message;
this.statusCode = statusCode;
Error.captureStackTrace(this, this.constructor);
}

toJSON(): object {
return {
isJoi: true,
name: this.constructor.name,
details: [
{
message: this.message,
},
],
};
}
}

export default BaseError;
10 changes: 10 additions & 0 deletions src/lib/error/disabled-error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import BaseError from './base-error';

class DisabledError extends BaseError {
constructor(message: string) {
super(message, 422, 'DisabledError');
Error.captureStackTrace(this, this.constructor);
}
}

export default DisabledError;
10 changes: 10 additions & 0 deletions src/lib/error/password-mismatch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import BaseError from './base-error';

class PasswordMismatch extends BaseError {
constructor(message: string = 'Wrong password, try again.') {
super(message, 401, 'PasswordMismatch');
Error.captureStackTrace(this, this.constructor);
}
}

export default PasswordMismatch;
1 change: 0 additions & 1 deletion src/lib/routes/auth/reset-password-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,3 @@ class ResetPasswordController extends Controller {
}

export default ResetPasswordController;
module.exports = ResetPasswordController;
10 changes: 3 additions & 7 deletions src/lib/routes/auth/simple-password-provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,9 @@ class PasswordProvider extends Controller {
});
}

try {
const user = await this.userService.loginUser(username, password);
req.session.user = user;
return res.status(200).json(user);
} catch (e) {
return res.status(401).json({ message: e.message });
}
const user = await this.userService.loginUser(username, password);
req.session.user = user;
return res.status(200).json(user);
}
}

Expand Down
6 changes: 6 additions & 0 deletions src/lib/routes/util.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import joi from 'joi';
import { Response } from 'express';
import { Logger } from '../logger';
import BaseError from '../error/base-error';

export const customJoi = joi.extend((j) => ({
type: 'isUrlFriendly',
Expand Down Expand Up @@ -29,6 +30,11 @@ export const handleErrors: (
// @ts-ignore
// eslint-disable-next-line no-param-reassign
error.isJoi = true;

if (error instanceof BaseError) {
return res.status(error.statusCode).json(error).end();
}

switch (error.name) {
case 'ValidationError':
return res.status(400).json(error).end();
Expand Down
8 changes: 5 additions & 3 deletions src/lib/services/user-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import { RoleName } from '../types/model';
import SettingService from './setting-service';
import { SimpleAuthSettings } from '../server-impl';
import { simpleAuthKey } from '../types/settings/simple-auth-settings';
import DisabledError from '../error/disabled-error';
import PasswordMismatch from '../error/password-mismatch';

const systemUser = new User({ id: -1, username: 'system' });

Expand Down Expand Up @@ -273,8 +275,8 @@ class UserService {
simpleAuthKey,
);

if (settings && settings.disabled) {
throw new Error(
if (settings?.disabled) {
throw new DisabledError(
'Logging in with username/password has been disabled.',
);
}
Expand All @@ -290,7 +292,7 @@ class UserService {
await this.store.successfullyLogin(user);
return user;
}
throw new Error('Wrong password, try again.');
throw new PasswordMismatch();
}

/**
Expand Down

0 comments on commit a50d0e2

Please sign in to comment.