Skip to content

Commit

Permalink
Merge branch '7.x' into backport/7.x/pr-64407
Browse files Browse the repository at this point in the history
  • Loading branch information
watson committed Apr 27, 2020
2 parents 6de9679 + 0e99bd6 commit 2fda298
Show file tree
Hide file tree
Showing 54 changed files with 706 additions and 566 deletions.
2 changes: 1 addition & 1 deletion docs/development/core/server/kibana-plugin-core-server.md
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ The plugin integrates with the core system via lifecycle events: `setup`<!-- -->
| [SavedObjectAttributeSingle](./kibana-plugin-core-server.savedobjectattributesingle.md) | Don't use this type, it's simply a helper type for [SavedObjectAttribute](./kibana-plugin-core-server.savedobjectattribute.md) |
| [SavedObjectMigrationFn](./kibana-plugin-core-server.savedobjectmigrationfn.md) | A migration function for a [saved object type](./kibana-plugin-core-server.savedobjectstype.md) used to migrate it to a given version |
| [SavedObjectSanitizedDoc](./kibana-plugin-core-server.savedobjectsanitizeddoc.md) | |
| [SavedObjectsClientContract](./kibana-plugin-core-server.savedobjectsclientcontract.md) | Saved Objects is Kibana's data persisentence mechanism allowing plugins to use Elasticsearch for storing plugin state.<!-- -->\#\# SavedObjectsClient errors<!-- -->Since the SavedObjectsClient has its hands in everything we are a little paranoid about the way we present errors back to to application code. Ideally, all errors will be either:<!-- -->1. Caused by bad implementation (ie. undefined is not a function) and as such unpredictable 2. An error that has been classified and decorated appropriately by the decorators in [SavedObjectsErrorHelpers](./kibana-plugin-core-server.savedobjectserrorhelpers.md)<!-- -->Type 1 errors are inevitable, but since all expected/handle-able errors should be Type 2 the <code>isXYZError()</code> helpers exposed at <code>SavedObjectsErrorHelpers</code> should be used to understand and manage error responses from the <code>SavedObjectsClient</code>.<!-- -->Type 2 errors are decorated versions of the source error, so if the elasticsearch client threw an error it will be decorated based on its type. That means that rather than looking for <code>error.body.error.type</code> or doing substring checks on <code>error.body.error.reason</code>, just use the helpers to understand the meaning of the error:<!-- -->\`\`\`<!-- -->js if (SavedObjectsErrorHelpers.isNotFoundError(error)) { // handle 404 }<!-- -->if (SavedObjectsErrorHelpers.isNotAuthorizedError(error)) { // 401 handling should be automatic, but in case you wanted to know }<!-- -->// always rethrow the error unless you handle it throw error; \`\`\`<!-- -->\#\#\# 404s from missing index<!-- -->From the perspective of application code and APIs the SavedObjectsClient is a black box that persists objects. One of the internal details that users have no control over is that we use an elasticsearch index for persistance and that index might be missing.<!-- -->At the time of writing we are in the process of transitioning away from the operating assumption that the SavedObjects index is always available. Part of this transition is handling errors resulting from an index missing. These used to trigger a 500 error in most cases, and in others cause 404s with different error messages.<!-- -->From my (Spencer) perspective, a 404 from the SavedObjectsApi is a 404; The object the request/call was targeting could not be found. This is why \#14141 takes special care to ensure that 404 errors are generic and don't distinguish between index missing or document missing.<!-- -->\#\#\# 503s from missing index<!-- -->Unlike all other methods, create requests are supposed to succeed even when the Kibana index does not exist because it will be automatically created by elasticsearch. When that is not the case it is because Elasticsearch's <code>action.auto_create_index</code> setting prevents it from being created automatically so we throw a special 503 with the intention of informing the user that their Elasticsearch settings need to be updated.<!-- -->See [SavedObjectsClient](./kibana-plugin-core-server.savedobjectsclient.md) See [SavedObjectsErrorHelpers](./kibana-plugin-core-server.savedobjectserrorhelpers.md) |
| [SavedObjectsClientContract](./kibana-plugin-core-server.savedobjectsclientcontract.md) | Saved Objects is Kibana's data persisentence mechanism allowing plugins to use Elasticsearch for storing plugin state.<!-- -->\#\# SavedObjectsClient errors<!-- -->Since the SavedObjectsClient has its hands in everything we are a little paranoid about the way we present errors back to to application code. Ideally, all errors will be either:<!-- -->1. Caused by bad implementation (ie. undefined is not a function) and as such unpredictable 2. An error that has been classified and decorated appropriately by the decorators in [SavedObjectsErrorHelpers](./kibana-plugin-core-server.savedobjectserrorhelpers.md)<!-- -->Type 1 errors are inevitable, but since all expected/handle-able errors should be Type 2 the <code>isXYZError()</code> helpers exposed at <code>SavedObjectsErrorHelpers</code> should be used to understand and manage error responses from the <code>SavedObjectsClient</code>.<!-- -->Type 2 errors are decorated versions of the source error, so if the elasticsearch client threw an error it will be decorated based on its type. That means that rather than looking for <code>error.body.error.type</code> or doing substring checks on <code>error.body.error.reason</code>, just use the helpers to understand the meaning of the error:<!-- -->\`\`\`<!-- -->js if (SavedObjectsErrorHelpers.isNotFoundError(error)) { // handle 404 }<!-- -->if (SavedObjectsErrorHelpers.isNotAuthorizedError(error)) { // 401 handling should be automatic, but in case you wanted to know }<!-- -->// always rethrow the error unless you handle it throw error; \`\`\`<!-- -->\#\#\# 404s from missing index<!-- -->From the perspective of application code and APIs the SavedObjectsClient is a black box that persists objects. One of the internal details that users have no control over is that we use an elasticsearch index for persistance and that index might be missing.<!-- -->At the time of writing we are in the process of transitioning away from the operating assumption that the SavedObjects index is always available. Part of this transition is handling errors resulting from an index missing. These used to trigger a 500 error in most cases, and in others cause 404s with different error messages.<!-- -->From my (Spencer) perspective, a 404 from the SavedObjectsApi is a 404; The object the request/call was targeting could not be found. This is why \#14141 takes special care to ensure that 404 errors are generic and don't distinguish between index missing or document missing.<!-- -->See [SavedObjectsClient](./kibana-plugin-core-server.savedobjectsclient.md) See [SavedObjectsErrorHelpers](./kibana-plugin-core-server.savedobjectserrorhelpers.md) |
| [SavedObjectsClientFactory](./kibana-plugin-core-server.savedobjectsclientfactory.md) | Describes the factory used to create instances of the Saved Objects Client. |
| [SavedObjectsClientFactoryProvider](./kibana-plugin-core-server.savedobjectsclientfactoryprovider.md) | Provider to invoke to retrieve a [SavedObjectsClientFactory](./kibana-plugin-core-server.savedobjectsclientfactory.md)<!-- -->. |
| [SavedObjectsClientWrapperFactory](./kibana-plugin-core-server.savedobjectsclientwrapperfactory.md) | Describes the factory used to create instances of Saved Objects Client Wrappers. |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ At the time of writing we are in the process of transitioning away from the oper

From my (Spencer) perspective, a 404 from the SavedObjectsApi is a 404; The object the request/call was targeting could not be found. This is why \#14141 takes special care to ensure that 404 errors are generic and don't distinguish between index missing or document missing.

\#\#\# 503s from missing index

Unlike all other methods, create requests are supposed to succeed even when the Kibana index does not exist because it will be automatically created by elasticsearch. When that is not the case it is because Elasticsearch's `action.auto_create_index` setting prevents it from being created automatically so we throw a special 503 with the intention of informing the user that their Elasticsearch settings need to be updated.

See [SavedObjectsClient](./kibana-plugin-core-server.savedobjectsclient.md) See [SavedObjectsErrorHelpers](./kibana-plugin-core-server.savedobjectserrorhelpers.md)

<b>Signature:</b>
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ export declare class SavedObjectsErrorHelpers
| --- | --- | --- |
| [createBadRequestError(reason)](./kibana-plugin-core-server.savedobjectserrorhelpers.createbadrequesterror.md) | <code>static</code> | |
| [createConflictError(type, id)](./kibana-plugin-core-server.savedobjectserrorhelpers.createconflicterror.md) | <code>static</code> | |
| [createEsAutoCreateIndexError()](./kibana-plugin-core-server.savedobjectserrorhelpers.createesautocreateindexerror.md) | <code>static</code> | |
| [createGenericNotFoundError(type, id)](./kibana-plugin-core-server.savedobjectserrorhelpers.creategenericnotfounderror.md) | <code>static</code> | |
| [createInvalidVersionError(versionInput)](./kibana-plugin-core-server.savedobjectserrorhelpers.createinvalidversionerror.md) | <code>static</code> | |
| [createUnsupportedTypeError(type)](./kibana-plugin-core-server.savedobjectserrorhelpers.createunsupportedtypeerror.md) | <code>static</code> | |
Expand All @@ -31,7 +30,6 @@ export declare class SavedObjectsErrorHelpers
| [decorateRequestEntityTooLargeError(error, reason)](./kibana-plugin-core-server.savedobjectserrorhelpers.decoraterequestentitytoolargeerror.md) | <code>static</code> | |
| [isBadRequestError(error)](./kibana-plugin-core-server.savedobjectserrorhelpers.isbadrequesterror.md) | <code>static</code> | |
| [isConflictError(error)](./kibana-plugin-core-server.savedobjectserrorhelpers.isconflicterror.md) | <code>static</code> | |
| [isEsAutoCreateIndexError(error)](./kibana-plugin-core-server.savedobjectserrorhelpers.isesautocreateindexerror.md) | <code>static</code> | |
| [isEsCannotExecuteScriptError(error)](./kibana-plugin-core-server.savedobjectserrorhelpers.isescannotexecutescripterror.md) | <code>static</code> | |
| [isEsUnavailableError(error)](./kibana-plugin-core-server.savedobjectserrorhelpers.isesunavailableerror.md) | <code>static</code> | |
| [isForbiddenError(error)](./kibana-plugin-core-server.savedobjectserrorhelpers.isforbiddenerror.md) | <code>static</code> | |
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
"**/@types/hapi": "^17.0.18",
"**/@types/angular": "^1.6.56",
"**/@types/hoist-non-react-statics": "^3.3.1",
"**/@types/chai": "^4.2.11",
"**/typescript": "3.7.2",
"**/graphql-toolkit/lodash": "^4.17.13",
"**/hoist-non-react-statics": "^3.3.2",
Expand Down
18 changes: 1 addition & 17 deletions src/core/public/saved_objects/saved_objects_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,17 +216,7 @@ export class SavedObjectsClient {
}),
});

return createRequest
.then(resp => this.createSavedObject(resp))
.catch((error: object) => {
if (isAutoCreateIndexError(error)) {
window.location.assign(
this.http.basePath.prepend('/app/kibana#/error/action.auto_create_index')
);
}

throw error;
});
return createRequest.then(resp => this.createSavedObject(resp));
};

/**
Expand Down Expand Up @@ -468,9 +458,3 @@ const renameKeys = <T extends Record<string, any>, U extends Record<string, any>
...{ [keysMap[key] || key]: obj[key] },
};
}, {});

const isAutoCreateIndexError = (error: any) => {
return (
error?.res?.status === 503 && error?.body?.attributes?.code === 'ES_AUTO_CREATE_INDEX_ERROR'
);
};
39 changes: 0 additions & 39 deletions src/core/server/saved_objects/service/lib/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -403,43 +403,4 @@ describe('savedObjectsClient/errorTypes', () => {
});
});
});

describe('EsAutoCreateIndex error', () => {
describe('createEsAutoCreateIndexError', () => {
it('does not take an error argument', () => {
const error = new Error();
// @ts-ignore
expect(SavedObjectsErrorHelpers.createEsAutoCreateIndexError(error)).not.toBe(error);
});

it('returns a new Error', () => {
expect(SavedObjectsErrorHelpers.createEsAutoCreateIndexError()).toBeInstanceOf(Error);
});

it('makes errors identifiable as EsAutoCreateIndex errors', () => {
expect(
SavedObjectsErrorHelpers.isEsAutoCreateIndexError(
SavedObjectsErrorHelpers.createEsAutoCreateIndexError()
)
).toBe(true);
});

it('returns a boom error', () => {
const error = SavedObjectsErrorHelpers.createEsAutoCreateIndexError();
expect(error).toHaveProperty('isBoom', true);
});

describe('error.output', () => {
it('uses "Automatic index creation failed" message', () => {
const error = SavedObjectsErrorHelpers.createEsAutoCreateIndexError();
expect(error.output.payload).toHaveProperty('message', 'Automatic index creation failed');
});

it('sets statusCode to 503', () => {
const error = SavedObjectsErrorHelpers.createEsAutoCreateIndexError();
expect(error.output).toHaveProperty('statusCode', 503);
});
});
});
});
});
14 changes: 0 additions & 14 deletions src/core/server/saved_objects/service/lib/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ const CODE_CONFLICT = 'SavedObjectsClient/conflict';
const CODE_ES_CANNOT_EXECUTE_SCRIPT = 'SavedObjectsClient/esCannotExecuteScript';
// 503 - Es Unavailable
const CODE_ES_UNAVAILABLE = 'SavedObjectsClient/esUnavailable';
// 503 - Unable to automatically create index because of action.auto_create_index setting
const CODE_ES_AUTO_CREATE_INDEX_ERROR = 'SavedObjectsClient/autoCreateIndex';
// 500 - General Error
const CODE_GENERAL_ERROR = 'SavedObjectsClient/generalError';

Expand Down Expand Up @@ -180,18 +178,6 @@ export class SavedObjectsErrorHelpers {
return isSavedObjectsClientError(error) && error[code] === CODE_ES_UNAVAILABLE;
}

public static createEsAutoCreateIndexError() {
const error = Boom.serverUnavailable('Automatic index creation failed');
error.output.payload.attributes = error.output.payload.attributes || {};
error.output.payload.attributes.code = 'ES_AUTO_CREATE_INDEX_ERROR';

return decorate(error, CODE_ES_AUTO_CREATE_INDEX_ERROR, 503);
}

public static isEsAutoCreateIndexError(error: Error | DecoratedError) {
return isSavedObjectsClientError(error) && error[code] === CODE_ES_AUTO_CREATE_INDEX_ERROR;
}

public static decorateGeneralError(error: Error, reason?: string) {
return decorate(error, CODE_GENERAL_ERROR, 500, reason);
}
Expand Down
53 changes: 22 additions & 31 deletions src/core/server/saved_objects/service/lib/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,40 +238,31 @@ export class SavedObjectsRepository {
}
}

try {
const migrated = this._migrator.migrateDocument({
id,
type,
...(savedObjectNamespace && { namespace: savedObjectNamespace }),
...(savedObjectNamespaces && { namespaces: savedObjectNamespaces }),
attributes,
migrationVersion,
updated_at: time,
...(Array.isArray(references) && { references }),
});

const raw = this._serializer.savedObjectToRaw(migrated as SavedObjectSanitizedDoc);
const migrated = this._migrator.migrateDocument({
id,
type,
...(savedObjectNamespace && { namespace: savedObjectNamespace }),
...(savedObjectNamespaces && { namespaces: savedObjectNamespaces }),
attributes,
migrationVersion,
updated_at: time,
...(Array.isArray(references) && { references }),
});

const method = id && overwrite ? 'index' : 'create';
const response = await this._writeToCluster(method, {
id: raw._id,
index: this.getIndexForType(type),
refresh,
body: raw._source,
});
const raw = this._serializer.savedObjectToRaw(migrated as SavedObjectSanitizedDoc);

return this._rawToSavedObject<T>({
...raw,
...response,
});
} catch (error) {
if (SavedObjectsErrorHelpers.isNotFoundError(error)) {
// See "503s from missing index" above
throw SavedObjectsErrorHelpers.createEsAutoCreateIndexError();
}
const method = id && overwrite ? 'index' : 'create';
const response = await this._writeToCluster(method, {
id: raw._id,
index: this.getIndexForType(type),
refresh,
body: raw._source,
});

throw error;
}
return this._rawToSavedObject<T>({
...raw,
...response,
});
}

/**
Expand Down
9 changes: 0 additions & 9 deletions src/core/server/saved_objects/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,6 @@ export type MutatingOperationRefreshSetting = boolean | 'wait_for';
* takes special care to ensure that 404 errors are generic and don't distinguish
* between index missing or document missing.
*
* ### 503s from missing index
*
* Unlike all other methods, create requests are supposed to succeed even when
* the Kibana index does not exist because it will be automatically created by
* elasticsearch. When that is not the case it is because Elasticsearch's
* `action.auto_create_index` setting prevents it from being created automatically
* so we throw a special 503 with the intention of informing the user that their
* Elasticsearch settings need to be updated.
*
* See {@link SavedObjectsClient}
* See {@link SavedObjectsErrorHelpers}
*
Expand Down
4 changes: 0 additions & 4 deletions src/core/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1875,8 +1875,6 @@ export class SavedObjectsErrorHelpers {
// (undocumented)
static createConflictError(type: string, id: string): DecoratedError;
// (undocumented)
static createEsAutoCreateIndexError(): DecoratedError;
// (undocumented)
static createGenericNotFoundError(type?: string | null, id?: string | null): DecoratedError;
// (undocumented)
static createInvalidVersionError(versionInput?: string): DecoratedError;
Expand All @@ -1903,8 +1901,6 @@ export class SavedObjectsErrorHelpers {
// (undocumented)
static isConflictError(error: Error | DecoratedError): boolean;
// (undocumented)
static isEsAutoCreateIndexError(error: Error | DecoratedError): boolean;
// (undocumented)
static isEsCannotExecuteScriptError(error: Error | DecoratedError): boolean;
// (undocumented)
static isEsUnavailableError(error: Error | DecoratedError): boolean;
Expand Down
4 changes: 2 additions & 2 deletions src/es_archiver/actions/rebuild_all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/

import { resolve, dirname, relative } from 'path';
import { stat, rename, createReadStream, createWriteStream } from 'fs';
import { stat, Stats, rename, createReadStream, createWriteStream } from 'fs';
import { Readable, Writable } from 'stream';
import { fromNode } from 'bluebird';
import { ToolingLog } from '@kbn/dev-utils';
Expand All @@ -33,7 +33,7 @@ import {
} from '../lib';

async function isDirectory(path: string): Promise<boolean> {
const stats = await fromNode(cb => stat(path, cb));
const stats: Stats = await fromNode(cb => stat(path, cb));
return stats.isDirectory();
}

Expand Down
Loading

0 comments on commit 2fda298

Please sign in to comment.