Skip to content

Commit

Permalink
Removed shouldRetry() method
Browse files Browse the repository at this point in the history
  • Loading branch information
@juanhapes
juanhapes committed Mar 11, 2022
1 parent 8b0650b commit b5ee53a
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 148 deletions.
30 changes: 6 additions & 24 deletions README.md
View file
Expand Up @@ -74,14 +74,6 @@ These methods **WILL NOT THROW AN ERROR** when response `statusCode` is `400+`.

### Extra

* `shouldRetry(response)` _Since 4.0.0_

Indicates if should re-try the call. It is useful for Event-Listeners API to avoid unnecessary retries.

Params: `response` `{MicroServiceCallResponse | MicroServiceCallError}`

Returns a `Boolean`.

* `static get errorCodes()` _Since 5.0.0_

Retrieves the MicroserviceCallError codes.
Expand Down Expand Up @@ -219,11 +211,6 @@ try {
statusCode: 404
}
*/

if(ms.shouldRetry(error)) // false
throw new Error('Should Retry')

// Do something
}
```
</details>
Expand Down Expand Up @@ -280,11 +267,6 @@ try {
statusCode: 500
}
*/

if(ms.shouldRetry(error)) // true
throw new Error('Service Call Fails. Should Retry')

// Do something
}
```

Expand Down Expand Up @@ -317,8 +299,8 @@ const response = await ms.safeCall('pricing', 'base-price', 'get', null, null, {
}
*/

if(ms.shouldRetry(response)) // true
throw new Error('Should Retry')
if(response.status >= 500) // true
throw new Error('Should Retry');

// Do something

Expand All @@ -338,8 +320,8 @@ const response = await ms.safeCall('wms', 'stock', 'post', { name: 'stock-1', qu
}
*/

if(ms.shouldRetry(response)) // false
throw new Error('Should Retry')
if(response.status >= 500) // false
throw new Error('Should Retry');

// Do something

Expand Down Expand Up @@ -390,8 +372,8 @@ const response = await ms.safeList('commerce', 'seller', { filters });
}
*/

if(ms.shouldRetry(error)) // false
throw new Error('Service Call Fails. Should Retry')
if(response.status >= 500) // false
throw new Error('Service Call Fails. Should Retry');

// Do something

Expand Down
56 changes: 51 additions & 5 deletions docs/migration-from-v4-to-v5.md
View file
Expand Up @@ -4,11 +4,12 @@ Version `5.0.0` radically changed the way the package communicates with services

API requests are no longer used, lambda invocations are now used with :package: [lambda](https://www.npmjs.com/package/@janiscommerce/lambda).

### Changes you must do
### Changes you **must** do

In case the service is using error codes, it should be modified as follows.

#### RouterFetcherError and ROUTER_FETCHER_ERROR_CODE
<details>
<summary>RouterFetcherError and ROUTER_FETCHER_ERROR_CODE</summary>

In `v4`...

Expand Down Expand Up @@ -50,7 +51,47 @@ try {
}
```

### Error codes changes
</details>

<details>
<summary>shouldRetry()</summary>

The method `shouldRetry()` was deleted completely.

In `v4`

```js

const MsCall = require('@janiscommerce/microservice-call');

const msCall = new MsCall();

const response = await msCall.safeCall('srv', 'np', 'md');

if(msCall.shouldRetry(response))
throw new Error('Retry');

```

In `v5`

```js

const MsCall = require('@janiscommerce/microservice-call');

const msCall = new MsCall();

const response = await msCall.safeCall('srv', 'np', 'md');

if(response.status >= 500)
throw new Error('Retry');

```

</details>

<details>
<summary>Error codes changes</summary>

In `v4` the :package: [router-fetcher](https://www.npmjs.com/package/@janiscommerce/router-fetcher) throws the following errors, in `v5` are thrown by `microservice-call`.

Expand All @@ -68,8 +109,13 @@ In `v4` the following errors were thrown, in `v5` are replaced or not be used an
| `REQUEST_LIB_ERROR` **3** | **Not exists anymore** | Request library errors |
| `JANIS_SECRET_MISSING` **4** | **Not exists anymore** | The Janis Secret is missing |

### Changed the default pageSize :warning:
</details>

<details>
<summary>Changed the default pageSize :warning:</summary>

Since lambda function are now used, the default page size is now **1.000** for the methods `list()` and `safeList()`.

This will not break your code, but is important to beware this change.
This will not break your code, but is important to beware this change.

</details>
46 changes: 0 additions & 46 deletions lib/ms-call.js
View file
Expand Up @@ -6,11 +6,6 @@ const MicroServiceCallError = require('./ms-call-error');

const EndpointFetcher = require('./endpoint-fetcher');

const NoRetryErrorMessages = [
': Argument passed in must be a single String of 12 bytes or a string of 24 hex characters',
': Invalid client'
];

const DEFAULT_LIST_PAGE_SIZE = 1000;

const HEADER_PAGE_SIZE = 'x-janis-page-size';
Expand Down Expand Up @@ -185,45 +180,4 @@ module.exports = class MsCall {
...body && { body }
};
}

/**
* Indicates if should re-try the call
*
* @param {RequestResponse|MicroServiceCallError} response MicroService Response or Error
* @returns {boolean}
*/
shouldRetry(response = {}) {

if(!response.statusCode)
return true;

const message = response instanceof MicroServiceCallError ?
this.getMessageFromError(response) :
this.getMessageFromResponse(response);

return response.statusCode >= 500 && !NoRetryErrorMessages.includes(message);
}

/**
* Get the message from an MsCall Error
*
* @private
* @param {MicroServiceCallError} error MicroServiceCallError
* @returns {string}
*/
getMessageFromError(error) {
const [, , message] = error.message.split(/[()]/g);
return message;
}

/**
* Get the message from a MsCall Response
*
* @private
* @param {RequestResponse} response MicroService Response
* @returns {string}
*/
getMessageFromResponse(response) {
return (response.body && response.body.message) ? `: ${response.body.message}` : '';
}
};
73 changes: 0 additions & 73 deletions tests/ms-call.js
View file
Expand Up @@ -14,7 +14,6 @@ const { Invoker } = require('@janiscommerce/lambda');
const MsCall = require('../lib/ms-call');
const EndpointFetcher = require('../lib/endpoint-fetcher');
const MsCallError = require('../lib/ms-call-error');
const MicroServiceCallError = require('../lib/ms-call-error');

describe('MsCall', () => {

Expand Down Expand Up @@ -433,78 +432,6 @@ describe('MsCall', () => {

});

describe('Using shouldRetry()', () => {

const msCall = new MsCall();

it('Should return true if no statusCode was found in response', () => {
assert(msCall.shouldRetry());
assert(msCall.shouldRetry({ body: [] }));
assert(msCall.shouldRetry(new Error('TypeError')));
assert(msCall.shouldRetry(new MicroServiceCallError('Microservice fails', MsCall.errorCodes.MICROSERVICE_FAILED)));
});

it('Should return true if statusCode is 500 and not an exceptional one', () => {

assert(msCall.shouldRetry(new MicroServiceCallError(
'Microservice failed (504): Timeout',
MicroServiceCallError.codes.MICROSERVICE_FAILED,
504
)));

assert(msCall.shouldRetry({
statusCode: 504,
body: { message: 'Timeout' }

}));
assert(msCall.shouldRetry({
statusCode: 500
}));
});

it('Should return false if statusCode is 4xx', () => {

assert(!msCall.shouldRetry(new MicroServiceCallError(
'Microservice failed (404): Not Found',
MicroServiceCallError.codes.MICROSERVICE_FAILED,
404
)));

assert(!msCall.shouldRetry({
statusCode: 404,
body: { message: 'Not Found' }
}));

assert(!msCall.shouldRetry({
statusCode: 404
}));
});

it('Should return false if statusCode is 500 and is an exceptional one', () => {

assert(!msCall.shouldRetry(new MicroServiceCallError(
'Microservice failed (500): Argument passed in must be a single String of 12 bytes or a string of 24 hex characters',
MicroServiceCallError.codes.MICROSERVICE_FAILED,
500
)));
assert(!msCall.shouldRetry({
statusCode: 500,
body: { message: 'Argument passed in must be a single String of 12 bytes or a string of 24 hex characters' }
}));

assert(!msCall.shouldRetry(new MicroServiceCallError(
'Microservice failed (500): Invalid client',
MicroServiceCallError.codes.MICROSERVICE_FAILED,
500
)));
assert(!msCall.shouldRetry({
statusCode: 500,
body: { message: 'Invalid client' }
}));
});

});

describe('EndpointFetcher', () => {

afterEach(() => sinon.assert.calledOnce(Settings.get));
Expand Down

0 comments on commit b5ee53a

Please sign in to comment.