[Ingest Manager] Split Registry errors into Connection & Response #76558

jfsiii · 2020-09-02T19:32:46Z

Summary

Pulled out of #74409

Add RegistryResponseError for !response.ok responses from the Registry like 4xx/5xx
Add RegistryConnectionError for a failure due to exhausted retries of system errors
Everything else that fails trying to connect continues to be a RegistryError
Change error messages

RegistryResponseError in master vs PR

- "Error connecting to package registry at http://localhost:8080/search?package=aws&internal=true&experimental=true: Error connecting to package registry at http://localhost:8080/search?package=aws&internal=true&experimental=true: Too Many Requests"
+ "'429 Too Many Requests' error response from package registry at http://localhost:8080/search?package=aws&internal=true&experimental=true"

RegistryConnectionError in master vs PR

- "Error connecting to package registry at http://localhost:8080/search?package=aws&internal=true&experimental=true: request to http://localhost:8080/search?package=aws&internal=true&experimental=true failed, reason: connect ECONNREFUSED 127.0.0.1:8080"
+ "Error connecting to package registry: request to http://localhost:8080/package/system/0.5.3 failed, reason: connect ECONNREFUSED 127.0.0.1:8080"

Checklist

Unit or functional tests were updated or added to match the most common scenarios

  setupIngestManager
    fetchUrl / getResponse errors
      ✓ regular Errors do not retry. Becomes RegistryError (8ms)
      ✓ TypeErrors do not retry. Becomes RegistryError (1ms)
      only system errors retry (like ECONNRESET)
        ✓ they eventually succeed (7012ms)
        ✓ or error after 1 failure & 5 retries with RegistryConnectionError (31020ms)
      4xx or 5xx from Registry become RegistryResponseError
        ✓ 404 (2ms)
        ✓ 429 (2ms)
        ✓ 500 (2ms)
      url in RegistryResponseError message is response.url || requested_url
        ✓ given response.url, use that (2ms)
        ✓ no response.url, use requested url (1ms)

jfsiii · 2020-09-02T20:40:14Z

x-pack/plugins/ingest_manager/server/services/epm/registry/requests.test.ts

@@ -53,13 +53,7 @@ describe('setupIngestManager', () => {
            throw new FetchError('message 3', 'system', { code: 'ESOMETHING' });
          })
          // this one succeeds
-          .mockImplementationOnce(() => Promise.resolve(new Response(successValue)))
-          .mockImplementationOnce(() => {


Leaving these extra mock responses was messing up counts in later tests. Confirmed real behavior was expected. Assume issue is in clearing mocks, but decided to just drop these.

elasticmachine · 2020-09-02T21:32:19Z

Pinging @elastic/ingest-management (Team:Ingest Management)

jfsiii · 2020-09-03T00:39:29Z

x-pack/plugins/ingest_manager/server/errors.ts

-  if (error instanceof PackageOutdatedError) {
-    return 400;
-  } else {
-    return 400; // Bad Request


If and else both returned 400, so I combined them

kibanamachine · 2020-09-03T03:18:14Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 9dbabad

Build metrics

✅ unchanged

History

💚 Build #72025 succeeded 46067c4

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

ruflin · 2020-09-03T06:45:22Z

These error message are much nicer. Do we log these errors into the Kibana log file?

…nes/processors-forms-k-s * 'master' of github.com:elastic/kibana: (216 commits) [Ingest Manager] Split Registry errors into Connection & Response (elastic#76558) [Security Solution] add an excess validation instead of the exact match (elastic#76472) Introduce TS incremental builds & move src/test_utils to TS project (elastic#76082) fix bad merge (elastic#76629) [Newsfeed] Ensure the version format when calling the API (elastic#76381) remove server_extensions mixin (elastic#76606) Remove legacy applications and legacy mode (elastic#75987) [Discover] Fix sidebar element focus behavior when adding / removing columns (elastic#75749) Replace FetchOptions with ISearchOptions (elastic#76538) Move streams utils to the core (elastic#76397) [Ingest Manager] Wording & linking improvements (elastic#76284) remove legacy kibana plugin (elastic#76064) [Form lib] Fix regression on field not being validated after reset to its default value. (elastic#76379) Legacy SO import: Fix bug causing multiple overrides to only show the last confirm modal (elastic#76482) [APM] Service maps layout enhancements (elastic#76481) [Security Solution][Detection Engine] Remove RuleTypeSchema in favor of Type for TypeScript (elastic#76586) [Security Solution][Exceptions] - Updates enum schema and tests (elastic#76544) Index Pattern class - remove toJSON and toString (elastic#76246) skip failing suite (elastic#76581) Split up and clarify Enterprise Search codeowners (elastic#76580) ... # Conflicts: # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processor_settings_fields.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/foreach.tsx

…oleysens/kibana into ingest-pipelines/processors-forms-k-s * 'ingest-pipelines/processors-forms-k-s' of github.com:jloleysens/kibana: (216 commits) [Ingest Manager] Split Registry errors into Connection & Response (elastic#76558) [Security Solution] add an excess validation instead of the exact match (elastic#76472) Introduce TS incremental builds & move src/test_utils to TS project (elastic#76082) fix bad merge (elastic#76629) [Newsfeed] Ensure the version format when calling the API (elastic#76381) remove server_extensions mixin (elastic#76606) Remove legacy applications and legacy mode (elastic#75987) [Discover] Fix sidebar element focus behavior when adding / removing columns (elastic#75749) Replace FetchOptions with ISearchOptions (elastic#76538) Move streams utils to the core (elastic#76397) [Ingest Manager] Wording & linking improvements (elastic#76284) remove legacy kibana plugin (elastic#76064) [Form lib] Fix regression on field not being validated after reset to its default value. (elastic#76379) Legacy SO import: Fix bug causing multiple overrides to only show the last confirm modal (elastic#76482) [APM] Service maps layout enhancements (elastic#76481) [Security Solution][Detection Engine] Remove RuleTypeSchema in favor of Type for TypeScript (elastic#76586) [Security Solution][Exceptions] - Updates enum schema and tests (elastic#76544) Index Pattern class - remove toJSON and toString (elastic#76246) skip failing suite (elastic#76581) Split up and clarify Enterprise Search codeowners (elastic#76580) ...

* master: (340 commits) [data.search.SearchSource] Remove legacy ES client APIs. (elastic#75943) [release notes] automatically retry on Github API 5xx errors (elastic#76447) [es_ui_shared] Fix eslint exhaustive deps rule (elastic#76392) [i18n] Integrate 7.9.1 Translations (elastic#76391) [APM] Update aggregations to support script sources (elastic#76429) [Security Solution] Refactor Network Top Countries to use Search Strategy (elastic#76244) Document security settings available on ESS (elastic#76513) [Ingest Manager] Add input revision to the config send to the agent (elastic#76327) [DOCS] Identifies cloud settings for Monitoring (elastic#76579) [DOCS] Identifies Cloud settings in Dev Tools (elastic#76583) [Ingest Manager] Better default value for fleet long polling timeout (elastic#76393) [data.indexPatterns] Fix broken rollup index pattern creation (elastic#76593) [Ingest Manager] Split Registry errors into Connection & Response (elastic#76558) [Security Solution] add an excess validation instead of the exact match (elastic#76472) Introduce TS incremental builds & move src/test_utils to TS project (elastic#76082) fix bad merge (elastic#76629) [Newsfeed] Ensure the version format when calling the API (elastic#76381) remove server_extensions mixin (elastic#76606) Remove legacy applications and legacy mode (elastic#75987) [Discover] Fix sidebar element focus behavior when adding / removing columns (elastic#75749) ...

…6558) (#76636) * Split Registry errors into Connection & Response * Ensure a url in ResponseError message. Add tests # Conflicts: # x-pack/plugins/ingest_manager/server/errors.ts

…6558) (#76634) * Split Registry errors into Connection & Response * Ensure a url in ResponseError message. Add tests

jfsiii · 2020-09-03T23:27:40Z

@ruflin not directly in this PR, but they are in #74409 and will be in others

ghost · 2020-09-18T13:20:06Z

Hi @nchaulet

We have gone through the ticket and observed it required DEV_Validation to test the ticket .

Please let us know if anything is missing from our end.

Split Registry errors into Connection & Response

46067c4

jfsiii added the Team:Fleet Team label for Observability Data Collection Fleet team label Sep 2, 2020

jfsiii commented Sep 2, 2020

View reviewed changes

jfsiii marked this pull request as ready for review September 2, 2020 21:32

jfsiii requested a review from a team September 2, 2020 21:32

jfsiii added release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0 apm:test-plan-7.9.0 Test plan for 7.9 release v7.9.2 and removed apm:test-plan-7.9.0 Test plan for 7.9 release labels Sep 2, 2020

jfsiii commented Sep 3, 2020

View reviewed changes

Ensure a url in ResponseError message. Add tests

9dbabad

nchaulet approved these changes Sep 3, 2020

View reviewed changes

jfsiii merged commit 2db7895 into elastic:master Sep 3, 2020

This was referenced Sep 3, 2020

[7.x] [Ingest Manager] Split Registry errors into Connection & Response (#76558) #76634

Merged

[7.9] [Ingest Manager] Split Registry errors into Connection & Response (#76558) #76636

Merged

jfsiii pushed a commit that referenced this pull request Sep 3, 2020

[Ingest Manager] Split Registry errors into Connection & Response (#7…

cad95fc

…6558) (#76634) * Split Registry errors into Connection & Response * Ensure a url in ResponseError message. Add tests

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Ingest Manager] Split Registry errors into Connection & Response #76558

[Ingest Manager] Split Registry errors into Connection & Response #76558

jfsiii commented Sep 2, 2020 •

edited

Loading

jfsiii Sep 2, 2020 •

edited

Loading

elasticmachine commented Sep 2, 2020

jfsiii Sep 3, 2020

kibanamachine commented Sep 3, 2020

ruflin commented Sep 3, 2020

jfsiii commented Sep 3, 2020

ghost commented Sep 18, 2020

[Ingest Manager] Split Registry errors into Connection & Response #76558

[Ingest Manager] Split Registry errors into Connection & Response #76558

Conversation

jfsiii commented Sep 2, 2020 • edited Loading

Summary

Checklist

jfsiii Sep 2, 2020 • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Sep 2, 2020

jfsiii Sep 3, 2020

Choose a reason for hiding this comment

kibanamachine commented Sep 3, 2020

💚 Build Succeeded

Build metrics

History

ruflin commented Sep 3, 2020

jfsiii commented Sep 3, 2020

ghost commented Sep 18, 2020

jfsiii commented Sep 2, 2020 •

edited

Loading

jfsiii Sep 2, 2020 •

edited

Loading