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/UiSettings] ignore certain errors #13079

Merged
merged 27 commits into from
Aug 9, 2017

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Jul 24, 2017

In #9214 we started ignoring certain elasticsearch errors in the UiSettings service. In #12747 we moved to using the SavedObjectsClient for communication with elasticsearch, which meant that we were getting different errors back and this broke the error ignoring characteristics of the UiSettings service.

The majority of the code is ready to go, but I need to write some tests that will fail in case this type of change happens again.

@spalger spalger force-pushed the fix/ui-settings/ignoreable-errors branch from 03e2e31 to a5892fc Compare July 24, 2017 21:58
@tylersmalley
Copy link
Contributor

Getting

server   error  [22:50:57.639]  TypeError: savedObjectsClient.errors.isNotFound is not a function
    at isIgnorableError (/Users/tyler/elastic/kibana/src/ui/ui_settings/ui_settings_service.js:110:33)
    at /Users/tyler/elastic/kibana/src/ui/ui_settings/ui_settings_service.js:120:11
    at throw (native)
    at step (/Users/tyler/elastic/kibana/src/ui/ui_settings/ui_settings_service.js:12:191)
    at /Users/tyler/elastic/kibana/src/ui/ui_settings/ui_settings_service.js:12:402

@spalger spalger force-pushed the fix/ui-settings/ignoreable-errors branch from 8618519 to e9ce550 Compare July 25, 2017 04:51
@spalger spalger force-pushed the fix/ui-settings/ignoreable-errors branch from e9ce550 to 304fd9e Compare July 25, 2017 05:08
Copy link
Contributor

@tylersmalley tylersmalley left a comment

Choose a reason for hiding this comment

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

LGTM - I like the file layout you ended up with in client/lib/errors.js

Copy link
Contributor

@tylersmalley tylersmalley left a comment

Choose a reason for hiding this comment

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

Came back to my instance this morning and the logs were flooded with the following:

server    log   [07:56:57.158] [warning][process] Error: Cannot provide statusCode or message with boom error
    at Object.exports.assert (/Users/tyler/elastic/kibana/node_modules/hoek/lib/index.js:736:11)
    at Object.exports.wrap (/Users/tyler/elastic/kibana/node_modules/boom/lib/index.js:96:10)
    at uiSettings.set.then.catch.err (/Users/tyler/elastic/kibana/src/core_plugins/kibana/server/routes/api/settings/register_set.js:18:34)

@spalger spalger force-pushed the fix/ui-settings/ignoreable-errors branch from dcd46d8 to 256a0b2 Compare July 31, 2017 18:45
@spalger
Copy link
Contributor Author

spalger commented Jul 31, 2017

@tylersmalley fixed the tests

Copy link
Contributor

@tylersmalley tylersmalley left a comment

Choose a reason for hiding this comment

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

LGTM. Also verified errors are now bubbling up for invalid login attempts.

@spalger spalger merged commit 8a64872 into elastic:master Aug 9, 2017
@spalger spalger deleted the fix/ui-settings/ignoreable-errors branch August 9, 2017 00:55
spalger added a commit that referenced this pull request Aug 9, 2017
* [SavedObjectClient] emit detectable errors

* [uiSettingsService] consume new SavedObjectsClient errors

* [SavedObjectsClient] expose errorTypeHelpers as such

* [elasticsearch/tests] recreate error for each test

* [http] wait for elasticsearch plugin to be ready

* [shortUrl/tests] ensure that create request responds with 200

* [shortUrl] use errorTypeHelpers to filter errors

* [uiSettings/savedObjectsClientStub] stub errorTypeHelpers

* [SavedObjectsClient/errors] expose error module so tests can make errors

* [shortUrl/tests] use actual SavedObjectsClient errors

* [uiSettings/savedObjectsClientStub] use actual errors lib

* [SavedObjectsClient] use decorate instead of "wrap"

* [server/routes/uiSettings] refactor routes to forward Boom errors from uiSettings

* [uiSettings] colocate routes and service

* [testUtils/esTestCluster] use more standard api style

* [testUtils/es] add createCallCluster util

* [testUtils/esTestCluster] add getters for client/callCluster

* [es/healthcheck] ensure that healtcheck stops when server is stopped

* [uiSettings/routes] add param/payload validation

* [uiSettings/routes] add tests that verify error behaviors

(cherry picked from commit 8a64872)
spalger added a commit that referenced this pull request Aug 9, 2017
* [SavedObjectClient] emit detectable errors

* [uiSettingsService] consume new SavedObjectsClient errors

* [SavedObjectsClient] expose errorTypeHelpers as such

* [elasticsearch/tests] recreate error for each test

* [http] wait for elasticsearch plugin to be ready

* [shortUrl/tests] ensure that create request responds with 200

* [shortUrl] use errorTypeHelpers to filter errors

* [uiSettings/savedObjectsClientStub] stub errorTypeHelpers

* [SavedObjectsClient/errors] expose error module so tests can make errors

* [shortUrl/tests] use actual SavedObjectsClient errors

* [uiSettings/savedObjectsClientStub] use actual errors lib

* [SavedObjectsClient] use decorate instead of "wrap"

* [server/routes/uiSettings] refactor routes to forward Boom errors from uiSettings

* [uiSettings] colocate routes and service

* [testUtils/esTestCluster] use more standard api style

* [testUtils/es] add createCallCluster util

* [testUtils/esTestCluster] add getters for client/callCluster

* [es/healthcheck] ensure that healtcheck stops when server is stopped

* [uiSettings/routes] add param/payload validation

* [uiSettings/routes] add tests that verify error behaviors

(cherry picked from commit 8a64872)
@spalger
Copy link
Contributor Author

spalger commented Aug 9, 2017

5.6: #13409
6.0: 1ba2c3f
6.1/6.x: 252024d

spalger added a commit to spalger/kibana that referenced this pull request Aug 9, 2017
spalger added a commit that referenced this pull request Aug 22, 2017
* [fix/UiSettings] ignore certain errors (#13079)

* [savedObjects/mappingFallback] check for body.error.type

* [savedObjectsClient] decorate non-es errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants