Skip to content
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.

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

fix: return 404 when a model was not found #1658

Merged
merged 7 commits into from
Sep 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 29 additions & 0 deletions docs/site/Error-handling.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
lang: en
title: Error Handling
keywords: LoopBack 4.0
sidebar: lb4_sidebar
permalink: /doc/en/lb4/Error-handling.html
---

## Known Error Codes

In order to allow clients to reliably detect individual error causes, LoopBack
sets the error `code` property to a machine-readable string.

| Error code | Description |
| :------------------------- | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| ENTITY_NOT_FOUND | The entity (model) was not found. This error is returned for example by [`EntityCrudRepository.prototype.findById`](http://apidocs.loopback.io/@loopback%2fdocs/repository.html#EntityCrudRepository.prototype.findById) |
| VALIDATION_FAILED | The data provided by the client is not a valid entity. |
| INVALID_PARAMETER_VALUE | The value provided for a parameter of a REST endpoint is not valid. For example, a string value was provided for a numeric parameter. |
| MISSING_REQUIRED_PARAMETER | No value was provided for a required parameter. |

Besides LoopBack-specific error codes, your application can encounter low-level
error codes from Node.js and the underlying operating system. For example, when
a connector is not able to reach the database, an error with code `ECONNREFUSED`
is returned.

See the following resources for a list of low-level error codes:

- [Common System Errors](https://nodejs.org/api/errors.html#errors_common_system_errors)
- [Node.js Error Codes](https://nodejs.org/api/errors.html#errors_node_js_error_codes)
4 changes: 4 additions & 0 deletions docs/site/sidebars/lb4_sidebar.yml
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,10 @@ children:
url: Decorators_repository.html
output: 'web, pdf'

- title: 'Error handling'
url: Error-handling.html
bajtos marked this conversation as resolved.
Show resolved Hide resolved
output: 'web, pdf'

- title: 'Booting an Application'
url: Booting-an-Application.html
output: 'web, pdf'
Expand Down
4 changes: 2 additions & 2 deletions examples/todo-list/src/controllers/todo-list.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import {Filter, Where, repository} from '@loopback/repository';
import {post, param, get, patch, del, requestBody} from '@loopback/rest';
import {Filter, repository, Where} from '@loopback/repository';
import {del, get, param, patch, post, requestBody} from '@loopback/rest';
import {TodoList} from '../models';
import {TodoListRepository} from '../repositories';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import {EntityNotFoundError} from '@loopback/repository';
import {createClientForHandler, expect, supertest} from '@loopback/testlab';
import {TodoListApplication} from '../../src/application';
import {TodoList} from '../../src/models/';
Expand Down Expand Up @@ -91,6 +92,10 @@ describe('Application', () => {
expect(result.body).to.deepEqual(expected);
});

it('returns 404 when a todo does not exist', () => {
return client.get('/todo-lists/99999').expect(404);
});

it('updates a todoList by ID', async () => {
const updatedTodoList = givenTodoList({
title: 'A different title to the todo list',
Expand All @@ -110,7 +115,7 @@ describe('Application', () => {
.expect(200);
await expect(
todoListRepo.findById(persistedTodoList.id),
).to.be.rejectedWith(/no TodoList found with id/);
).to.be.rejectedWith(EntityNotFoundError);
});
});

Expand Down
7 changes: 6 additions & 1 deletion examples/todo-list/test/acceptance/todo.acceptance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import {EntityNotFoundError} from '@loopback/repository';
import {createClientForHandler, expect, supertest} from '@loopback/testlab';
import {TodoListApplication} from '../../src/application';
import {Todo} from '../../src/models/';
Expand Down Expand Up @@ -63,6 +64,10 @@ describe('Application', () => {
expect(result.body).to.deepEqual(expected);
});

it('returns 404 when a todo does not exist', () => {
return client.get('/todos/99999').expect(404);
});

it('replaces the todo by ID', async () => {
const updatedTodo = givenTodo({
title: 'DO SOMETHING AWESOME',
Expand Down Expand Up @@ -96,7 +101,7 @@ describe('Application', () => {
.send()
.expect(200);
await expect(todoRepo.findById(persistedTodo.id)).to.be.rejectedWith(
/no Todo found with id/,
EntityNotFoundError,
);
});
});
Expand Down
9 changes: 7 additions & 2 deletions examples/todo/test/acceptance/todo.acceptance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,17 @@
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import {EntityNotFoundError} from '@loopback/repository';
import {createClientForHandler, expect, supertest} from '@loopback/testlab';
import {TodoListApplication} from '../../src/application';
import {Todo} from '../../src/models/';
import {TodoRepository} from '../../src/repositories/';
import {
HttpCachingProxy,
aLocation,
getProxiedGeoCoderConfig,
givenCachingProxy,
givenTodo,
HttpCachingProxy,
} from '../helpers';

describe('Application', () => {
Expand Down Expand Up @@ -95,6 +96,10 @@ describe('Application', () => {
expect(result.body).to.deepEqual(expected);
});

it('returns 404 when a todo does not exist', () => {
return client.get('/todos/99999').expect(404);
});

it('replaces the todo by ID', async () => {
const updatedTodo = givenTodo({
title: 'DO SOMETHING AWESOME',
Expand Down Expand Up @@ -128,7 +133,7 @@ describe('Application', () => {
.send()
.expect(200);
await expect(todoRepo.findById(persistedTodo.id)).to.be.rejectedWith(
/no Todo found with id/,
EntityNotFoundError,
);
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {Filter, Where, repository} from '@loopback/repository';
import {Filter, repository, Where} from '@loopback/repository';

import {
post,
param,
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/unit/update-index.unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('update-index unit tests', () => {
});

it('throws an error when given a non-ts file', async () => {
expect(updateIndex(SANDBOX_PATH, 'test.js')).to.be.rejectedWith(
await expect(updateIndex(SANDBOX_PATH, 'test.js')).to.be.rejectedWith(
/test.js must be a TypeScript \(.ts\) file/,
);
});
Expand Down
15 changes: 6 additions & 9 deletions packages/context/test/unit/value-promise.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,15 +244,14 @@ describe('resolveUntil', () => {
expect(result).to.eql('A');
});

it('handles a rejected promise from resolver', () => {
it('handles a rejected promise from resolver', async () => {
const source = ['a', 'b', 'c'];
const result = resolveUntil<string, string>(
source[Symbol.iterator](),
v => Promise.reject(new Error(v)),
(s, v) => true,
);
// tslint:disable-next-line:no-floating-promises
expect(result).be.rejectedWith('a');
await expect(result).be.rejectedWith('a');
});

it('reports an error thrown from resolver', () => {
Expand All @@ -268,7 +267,7 @@ describe('resolveUntil', () => {
expect(task).throw('a');
});

it('handles a rejected promise from evaluator', () => {
it('handles a rejected promise from evaluator', async () => {
const source = ['a', 'b', 'c'];
const result = resolveUntil<string, string>(
source[Symbol.iterator](),
Expand All @@ -277,8 +276,7 @@ describe('resolveUntil', () => {
throw new Error(v);
},
);
// tslint:disable-next-line:no-floating-promises
expect(result).be.rejectedWith('A');
await expect(result).be.rejectedWith('A');
});

it('handles mixed value and promise items ', async () => {
Expand Down Expand Up @@ -341,12 +339,11 @@ describe('transformValueOrPromise', () => {
expect(result).to.eql('A');
});

it('handles a rejected promise from the transformer', () => {
it('handles a rejected promise from the transformer', async () => {
const result = transformValueOrPromise('a', v =>
Promise.reject(new Error(v)),
);
// tslint:disable-next-line:no-floating-promises
expect(result).be.rejectedWith('a');
await expect(result).be.rejectedWith('a');
});

it('handles an error thrown from the transformer', () => {
Expand Down
40 changes: 40 additions & 0 deletions packages/repository/src/errors/entity-not-found.error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright IBM Corp. 2018. All Rights Reserved.
// Node module: @loopback/repository
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import {Entity} from '../model';

export class EntityNotFoundError<ID, Props extends object = {}> extends Error {
code: string;
entityName: string;
entityId: ID;

constructor(
entityOrName: typeof Entity | string,
entityId: ID,
extraProperties?: Props,
) {
const entityName =
typeof entityOrName === 'string'
? entityOrName
: entityOrName.modelName || entityOrName.name;

const quotedId = JSON.stringify(entityId);

super(`Entity not found: ${entityName} with id ${quotedId}`);

Error.captureStackTrace(this, this.constructor);

this.code = 'ENTITY_NOT_FOUND';
this.entityName = entityName;
this.entityId = entityId;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we set this to quotedId or not?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. The purpose of quotedId is to have a value that's easy to parse in a string, to make it easy for readers to distinguish between e.g. a number 1 and a string "1".

Error properties like entityId and entityName can be serialized in different ways in the HTTP response. When using JSON, the distinction between different types (undefined, null, a number, a string, an object/array) is preserved.


Object.assign(this, extraProperties);
}
}

// tslint:disable-next-line:no-any
export function isEntityNotFoundError(e: any): e is EntityNotFoundError<any> {
return e instanceof EntityNotFoundError;
}
6 changes: 6 additions & 0 deletions packages/repository/src/errors/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Copyright IBM Corp. 2018. All Rights Reserved.
// Node module: @loopback/repository
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

export * from './entity-not-found.error';
1 change: 1 addition & 0 deletions packages/repository/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

export * from './connectors';
export * from './decorators';
export * from './errors';
export * from './mixins';
export * from './repositories';
export * from './types';
Expand Down
5 changes: 4 additions & 1 deletion packages/repository/src/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,10 @@ function asObject(value: any, options?: Options): any {
* Base class for models
*/
export abstract class Model {
static modelName: string;
static get modelName(): string {
return (this.definition && this.definition.name) || this.name;
}

static definition: ModelDefinition;

/**
Expand Down
16 changes: 8 additions & 8 deletions packages/repository/src/repositories/legacy-juggler-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,26 @@
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import * as legacy from 'loopback-datasource-juggler';

import * as assert from 'assert';
import {isPromiseLike} from '@loopback/context';
import * as assert from 'assert';
import * as legacy from 'loopback-datasource-juggler';
import {
Options,
AnyObject,
Command,
DataObject,
NamedParameters,
Options,
PositionalParameters,
DataObject,
} from '../common-types';
import {HasManyDefinition} from '../decorators/relation.decorator';
import {EntityNotFoundError} from '../errors';
import {Entity, ModelDefinition} from '../model';
import {Filter, Where} from '../query';
import {EntityCrudRepository} from './repository';
import {
createHasManyRepositoryFactory,
HasManyRepositoryFactory,
} from './relation.factory';
import {HasManyDefinition} from '../decorators/relation.decorator';
import {EntityCrudRepository} from './repository';

export namespace juggler {
export import DataSource = legacy.DataSource;
Expand Down Expand Up @@ -211,7 +211,7 @@ 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}"`);
throw new EntityNotFoundError(this.entityClass, id);
}
return this.toEntity(model);
}
Expand Down
Loading