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

…astic#76558)

* Split Registry errors into Connection & Response

* Ensure a url in ResponseError message. Add tests

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

@@ -17,15 +17,14 @@ export const getHTTPResponseCode = (error: IngestManagerError): number => {
  return 502; // Bad Gateway
  }
  if (error instanceof PackageNotFoundError) {
- return 404;
- }
- if (error instanceof PackageOutdatedError) {
- return 400;
- } else {
- return 400; // Bad Request
+ return 404; // Not Found
  }
+
+ return 400; // Bad Request
 };
 
 export class RegistryError extends IngestManagerError {}
+export class RegistryConnectionError extends RegistryError {}
+export class RegistryResponseError extends RegistryError {}
 export class PackageNotFoundError extends IngestManagerError {}
 export class PackageOutdatedError extends IngestManagerError {}

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

@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 import { fetchUrl } from './requests';
-import { RegistryError } from '../../../errors';
+import { RegistryError, RegistryConnectionError, RegistryResponseError } from '../../../errors';
 jest.mock('node-fetch');
 
 const { Response, FetchError } = jest.requireActual('node-fetch');
@@ -53,13 +53,7 @@ describe('setupIngestManager', () => {
  throw new FetchError('message 3', 'system', { code: 'ESOMETHING' });
  })
  // this one succeeds
- .mockImplementationOnce(() => Promise.resolve(new Response(successValue)))
- .mockImplementationOnce(() => {
- throw new FetchError('message 5', 'system', { code: 'ESOMETHING' });
- })
- .mockImplementationOnce(() => {
- throw new FetchError('message 6', 'system', { code: 'ESOMETHING' });
- });
+ .mockImplementationOnce(() => Promise.resolve(new Response(successValue)));
 
  const promise = fetchUrl('');
  await expect(promise).resolves.toEqual(successValue);
@@ -69,7 +63,7 @@ describe('setupIngestManager', () => {
  expect(actualResultsOrder).toEqual(['throw', 'throw', 'throw', 'return']);
  });
 
- it('or error after 1 failure & 5 retries with RegistryError', async () => {
+ it('or error after 1 failure & 5 retries with RegistryConnectionError', async () => {
  fetchMock
  .mockImplementationOnce(() => {
  throw new FetchError('message 1', 'system', { code: 'ESOMETHING' });
@@ -88,21 +82,93 @@ describe('setupIngestManager', () => {
  })
  .mockImplementationOnce(() => {
  throw new FetchError('message 6', 'system', { code: 'ESOMETHING' });
- })
- .mockImplementationOnce(() => {
- throw new FetchError('message 7', 'system', { code: 'ESOMETHING' });
- })
- .mockImplementationOnce(() => {
- throw new FetchError('message 8', 'system', { code: 'ESOMETHING' });
  });
 
  const promise = fetchUrl('');
- await expect(promise).rejects.toThrow(RegistryError);
+ await expect(promise).rejects.toThrow(RegistryConnectionError);
  // doesn't retry after 1 failure & 5 failed retries
  expect(fetchMock).toHaveBeenCalledTimes(6);
  const actualResultsOrder = fetchMock.mock.results.map(({ type }: { type: string }) => type);
  expect(actualResultsOrder).toEqual(['throw', 'throw', 'throw', 'throw', 'throw', 'throw']);
  });
  });
+
+ describe('4xx or 5xx from Registry become RegistryResponseError', () => {
+ it('404', async () => {
+ fetchMock.mockImplementationOnce(() => ({
+ ok: false,
+ status: 404,
+ statusText: 'Not Found',
+ url: 'https://example.com',
+ }));
+ const promise = fetchUrl('');
+ await expect(promise).rejects.toThrow(RegistryResponseError);
+ await expect(promise).rejects.toThrow(
+ `'404 Not Found' error response from package registry at https://example.com`
+ );
+ expect(fetchMock).toHaveBeenCalledTimes(1);
+ });
+
+ it('429', async () => {
+ fetchMock.mockImplementationOnce(() => ({
+ ok: false,
+ status: 429,
+ statusText: 'Too Many Requests',
+ url: 'https://example.com',
+ }));
+ const promise = fetchUrl('');
+ await expect(promise).rejects.toThrow(RegistryResponseError);
+ await expect(promise).rejects.toThrow(
+ `'429 Too Many Requests' error response from package registry at https://example.com`
+ );
+ expect(fetchMock).toHaveBeenCalledTimes(1);
+ });
+
+ it('500', async () => {
+ fetchMock.mockImplementationOnce(() => ({
+ ok: false,
+ status: 500,
+ statusText: 'Internal Server Error',
+ url: 'https://example.com',
+ }));
+ const promise = fetchUrl('');
+ await expect(promise).rejects.toThrow(RegistryResponseError);
+ await expect(promise).rejects.toThrow(
+ `'500 Internal Server Error' error response from package registry at https://example.com`
+ );
+ expect(fetchMock).toHaveBeenCalledTimes(1);
+ });
+ });
+
+ describe('url in RegistryResponseError message is response.url || requested_url', () => {
+ it('given response.url, use that', async () => {
+ fetchMock.mockImplementationOnce(() => ({
+ ok: false,
+ status: 404,
+ statusText: 'Not Found',
+ url: 'https://example.com/?from_response=true',
+ }));
+ const promise = fetchUrl('https://example.com/?requested=true');
+ await expect(promise).rejects.toThrow(RegistryResponseError);
+ await expect(promise).rejects.toThrow(
+ `'404 Not Found' error response from package registry at https://example.com/?from_response=true`
+ );
+ expect(fetchMock).toHaveBeenCalledTimes(1);
+ });
+
+ it('no response.url, use requested url', async () => {
+ fetchMock.mockImplementationOnce(() => ({
+ ok: false,
+ status: 404,
+ statusText: 'Not Found',
+ }));
+ const promise = fetchUrl('https://example.com/?requested=true');
+ await expect(promise).rejects.toThrow(RegistryResponseError);
+ await expect(promise).rejects.toThrow(
+ `'404 Not Found' error response from package registry at https://example.com/?requested=true`
+ );
+ expect(fetchMock).toHaveBeenCalledTimes(1);
+ });
+ });
  });
 });

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

@@ -7,7 +7,7 @@
 import fetch, { FetchError, Response } from 'node-fetch';
 import pRetry from 'p-retry';
 import { streamToString } from './streams';
-import { RegistryError } from '../../../errors';
+import { RegistryError, RegistryConnectionError, RegistryResponseError } from '../../../errors';
 
 type FailedAttemptErrors = pRetry.FailedAttemptError | FetchError | Error;
 
@@ -19,10 +19,13 @@ async function registryFetch(url: string) {
  return response;
  } else {
  // 4xx & 5xx responses
- // exit without retry & throw RegistryError
- throw new pRetry.AbortError(
- new RegistryError(`Error connecting to package registry at ${url}: ${response.statusText}`)
- );
+ const { status, statusText, url: resUrl } = response;
+ const message = `'${status} ${statusText}' error response from package registry at ${
+ resUrl || url
+ }`;
+ const responseError = new RegistryResponseError(message);
+
+ throw new pRetry.AbortError(responseError);
  }
 }
 
@@ -38,17 +41,24 @@ export async function getResponse(url: string): Promise<Response> {
  // and let the others through without retrying
  //
  // throwing in onFailedAttempt will abandon all retries & fail the request
- // we only want to retry system errors, so throw a RegistryError for everything else
+ // we only want to retry system errors, so re-throw for everything else
  if (!isSystemError(error)) {
- throw new RegistryError(
- `Error connecting to package registry at ${url}: ${error.message}`
- );
+ throw error;
  }
  },
  });
  return response;
- } catch (e) {
- throw new RegistryError(`Error connecting to package registry at ${url}: ${e.message}`);
+ } catch (error) {
+ // isSystemError here means we didn't succeed after max retries
+ if (isSystemError(error)) {
+ throw new RegistryConnectionError(`Error connecting to package registry: ${error.message}`);
+ }
+ // don't wrap our own errors
+ if (error instanceof RegistryError) {
+ throw error;
+ } else {
+ throw new RegistryError(error);
+ }
  }
 }