Skip to content

Commit

Permalink
fix: return 404 when a model was not found
Browse files Browse the repository at this point in the history
Modify repository method `findById` to set `code = 'MODEL_NOT_FOUND'`
on the error thrown when no model instance was found.

Modify the `reject` action to convert MODEL_NOT_FOUND code to response
status code 404.
  • Loading branch information
bajtos committed Aug 30, 2018
1 parent 0364b59 commit c355a2b
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 13 deletions.
2 changes: 1 addition & 1 deletion examples/todo-list/test/acceptance/todo-list.acceptance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ describe('Application', () => {
.expect(200);
await expect(
todoListRepo.findById(persistedTodoList.id),
).to.be.rejectedWith(/no TodoList found with id/);
).to.be.rejectedWith(/Model not found: TodoList with id/);
});
});

Expand Down
2 changes: 1 addition & 1 deletion examples/todo-list/test/acceptance/todo.acceptance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe('Application', () => {
.send()
.expect(200);
await expect(todoRepo.findById(persistedTodo.id)).to.be.rejectedWith(
/no Todo found with id/,
/Model not found: Todo with id/,
);
});
});
Expand Down
2 changes: 1 addition & 1 deletion examples/todo/test/acceptance/todo.acceptance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ describe('Application', () => {
.send()
.expect(200);
await expect(todoRepo.findById(persistedTodo.id)).to.be.rejectedWith(
/no Todo found with id/,
/Model not found: Todo with id/,
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,12 @@ export class DefaultCrudRepository<T extends Entity, ID>
this.modelClass.findById(id, filter, options),
);
if (!model) {
throw new Error(`no ${this.modelClass.name} found with id "${id}"`);
const quotedId = JSON.stringify(id);
const err: Error & {code?: string} = new Error(
`Model not found: ${this.modelClass.name} with id ${quotedId}`,
);
err.code = 'MODEL_NOT_FOUND';
throw err;
}
return this.toEntity(model);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('DefaultCrudRepository', () => {

class Note extends Entity {
static definition = new ModelDefinition({
name: 'note3',
name: 'Note3',
properties: {
title: 'string',
content: 'string',
Expand Down Expand Up @@ -281,12 +281,9 @@ describe('DefaultCrudRepository', () => {

it('throws if findById does not return a value', async () => {
const repo = new DefaultCrudRepository(Note, ds);
try {
await repo.findById(999999);
} catch (err) {
expect(err).to.match(/no Note was found with id/);
return;
}
throw new Error('No error was returned!');
return expect(repo.findById(999999)).to.be.rejectedWith({
message: 'Model not found: Note3 with id 999999',
code: 'MODEL_NOT_FOUND',
});
});
});
16 changes: 15 additions & 1 deletion packages/rest/src/providers/reject.provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ import {RestBindings} from '../keys';

const debug = require('debug')('loopback:rest:reject');

// TODO(bajtos) Make this mapping configurable at RestServer level,
// allow apps and extensions to contribute additional mappings.
const codeToStatusCodeMapping: {[key: string]: number} = {
MODEL_NOT_FOUND: 404,
};

export class RejectProvider implements Provider<Reject> {
constructor(
@inject(RestBindings.SequenceActions.LOG_ERROR)
Expand All @@ -22,7 +28,15 @@ export class RejectProvider implements Provider<Reject> {
}

action({request, response}: HandlerContext, error: Error) {
const err = <HttpError>error;
const err = error as HttpError & {code?: string};

if (!err.status && !err.statusCode && err.code) {
const customStatus = codeToStatusCodeMapping[err.code];
if (customStatus) {
err.statusCode = customStatus;
}
}

const statusCode = err.statusCode || err.status || 500;
writeErrorToResponse(response, err);

Expand Down
12 changes: 12 additions & 0 deletions packages/rest/test/unit/router/reject.provider.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,18 @@ describe('reject', () => {
expect(result).to.have.property('statusCode', 500);
});

it('converts error code MODEL_NOT_FOUND to status code 404', async () => {
const reject = new RejectProvider(noopLogger).value();

const notFoundError: Error & {code?: string} = new Error('not found');
notFoundError.code = 'MODEL_NOT_FOUND';

reject(contextStub, notFoundError);
const result = await contextStub.result;

expect(result.statusCode).to.equal(404);
});

it('logs the error', async () => {
const logger = sinon.spy() as LogError & SinonSpy;
const reject = new RejectProvider(logger).value();
Expand Down

0 comments on commit c355a2b

Please sign in to comment.