feat: Throw PoHTTPClientBindingError when server returns a 40X (#67)

So that the client won't attempt another delivery.

src/index.ts

@@ -1,3 +1,4 @@
 export * from './lib/client';
 export { default as PoHTTPError } from './lib/PoHTTPError';
 export { default as PoHTTPInvalidParcelError } from './lib/PoHTTPInvalidParcelError';
+export { default as PoHTTPClientBindingError } from './lib/PoHTTPClientBindingError';

src/lib/PoHTTPClientBindingError.ts

@@ -0,0 +1,6 @@
+import PoHTTPError from './PoHTTPError';
+
+/**
+ * PoHTTP server refused parcel delivery, claiming a protocol violation by this client.
+ */
+export default class PoHTTPClientBindingError extends PoHTTPError {}

src/lib/client.spec.ts

@@ -4,6 +4,7 @@ import bufferToArray from 'buffer-to-arraybuffer';
 
 import { expectPromiseToReject, getMockContext, getMockInstance, mockEnvVars } from './_test_utils';
 import { deliverParcel, DeliveryOptions } from './client';
+import PoHTTPClientBindingError from './PoHTTPClientBindingError';
 import PoHTTPError from './PoHTTPError';
 import PoHTTPInvalidParcelError from './PoHTTPInvalidParcelError';
 
@@ -203,14 +204,13 @@ describe('deliverParcel', () => {
       expect(postCall2Args[0]).toEqual(stubRedirectUrl);
     });
 
-    test('Responses with unsupported 3XX status codes should be returned as is', async () => {
+    test('Responses with unsupported 3XX status codes should result in errors', async () => {
       const redirectResponse = { ...stubRedirectResponse, status: 302 };
       stubAxiosPost.mockRejectedValueOnce({ response: redirectResponse });
 
-      const response = await deliverParcel(url, body);
-
-      expect(stubAxiosPost).toBeCalledTimes(1);
-      expect(response).toBe(redirectResponse);
+      await expect(deliverParcel(url, body)).rejects.toEqual(
+        new PoHTTPError('Failed to deliver parcel (HTTP 302)'),
+      );
     });
 
     test('Original arguments should be honored when following redirects', async () => {
@@ -272,7 +272,7 @@ describe('deliverParcel', () => {
     stubAxiosPost.mockRejectedValueOnce({ response: { status: 403 } });
 
     await expect(deliverParcel(url, body)).rejects.toEqual(
-      new PoHTTPInvalidParcelError('Server refused to accept parcel'),
+      new PoHTTPInvalidParcelError('Server rejected parcel'),
     );
   });
 
@@ -283,7 +283,30 @@ describe('deliverParcel', () => {
     stubAxiosPost.mockRejectedValueOnce({ response: { data: { message: reason }, status: 403 } });
 
     await expect(deliverParcel(url, body)).rejects.toEqual(
-      new PoHTTPInvalidParcelError(`Server refused to accept parcel: ${reason}`),
+      new PoHTTPInvalidParcelError(`Server rejected parcel: ${reason}`),
+    );
+  });
+
+  test('HTTP 40X should throw a PoHTTPClientBindingError', async () => {
+    // @ts-ignore
+    stubAxiosPost.mockReset();
+    stubAxiosPost.mockRejectedValueOnce({ response: { status: 400 } });
+
+    await expect(deliverParcel(url, body)).rejects.toEqual(
+      new PoHTTPClientBindingError('Server rejected request due to protocol violation (HTTP 400)'),
+    );
+  });
+
+  test('PoHTTPClientBindingError should mention the reason if available', async () => {
+    // @ts-ignore
+    stubAxiosPost.mockReset();
+    const reason = 'Denied';
+    stubAxiosPost.mockRejectedValueOnce({ response: { data: { message: reason }, status: 400 } });
+
+    await expect(deliverParcel(url, body)).rejects.toEqual(
+      new PoHTTPClientBindingError(
+        `Server rejected request due to protocol violation (HTTP 400): ${reason}`,
+      ),
     );
   });
 
@@ -298,22 +321,22 @@ describe('deliverParcel', () => {
     await expectPromiseToReject(deliverParcel(url, body), expectedError);
   });
 
-  test('HTTP 4XX errors should mention the reason if available', async () => {
+  test('HTTP 5XX errors should mention the reason if available', async () => {
     // @ts-ignore
     stubAxiosPost.mockReset();
     const reason = 'Denied';
-    const status = 400;
+    const status = 500;
     stubAxiosPost.mockRejectedValueOnce({ response: { data: { message: reason }, status } });
 
     await expect(deliverParcel(url, body)).rejects.toEqual(
       new PoHTTPError(`Failed to deliver parcel (HTTP ${status}): ${reason}`),
     );
   });
 
-  test('HTTP 4XX errors should not mention the reason when absent', async () => {
+  test('HTTP 5XX errors should not mention the reason when absent', async () => {
     // @ts-ignore
     stubAxiosPost.mockReset();
-    const status = 400;
+    const status = 500;
     stubAxiosPost.mockRejectedValueOnce({ response: { data: {}, status } });
 
     await expect(deliverParcel(url, body)).rejects.toEqual(

src/lib/client.ts

@@ -3,6 +3,7 @@ import axios, { AxiosInstance, AxiosResponse } from 'axios';
 import { get as getEnvVar } from 'env-var';
 import * as https from 'https';
 
+import PoHTTPClientBindingError from './PoHTTPClientBindingError';
 import PoHTTPError from './PoHTTPError';
 import PoHTTPInvalidParcelError from './PoHTTPInvalidParcelError';
 
@@ -81,10 +82,17 @@ async function postRequest(
     const reason = error.response.data?.message;
     if (responseStatus === 403) {
       throw new PoHTTPInvalidParcelError(
-        reason ? `Server refused to accept parcel: ${reason}` : 'Server refused to accept parcel',
+        reason ? `Server rejected parcel: ${reason}` : 'Server rejected parcel',
       );
     }
-    if (responseStatus < 300 || 400 <= responseStatus) {
+    if (400 <= responseStatus && responseStatus < 500) {
+      throw new PoHTTPClientBindingError(
+        reason
+          ? `Server rejected request due to protocol violation (HTTP ${responseStatus}): ${reason}`
+          : `Server rejected request due to protocol violation (HTTP ${responseStatus})`,
+      );
+    }
+    if (!isStatusCodeValidRedirect(responseStatus) || 500 <= responseStatus) {
       throw new PoHTTPError(
         reason
           ? `Failed to deliver parcel (HTTP ${responseStatus}): ${reason}`
@@ -94,7 +102,7 @@ async function postRequest(
     response = error.response;
   }
 
-  if ((response.status === 307 || response.status === 308) && 0 < options.maxRedirects) {
+  if (isStatusCodeValidRedirect(response.status) && 0 < options.maxRedirects) {
     // Axios doesn't support 307 or 308 redirects: https://github.com/axios/axios/issues/2429,
     // so we have to follow the redirect manually.
     return postRequest(response.headers.location, body, axiosInstance, {
@@ -105,3 +113,7 @@ async function postRequest(
 
   return response;
 }
+
+function isStatusCodeValidRedirect(statusCode: number): boolean {
+  return statusCode === 307 || statusCode === 308;
+}