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

feat: make Request object thenable #515

Merged
merged 9 commits into from
Jul 26, 2024
110 changes: 53 additions & 57 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,19 +106,22 @@ Fetcher.registerService(myDataService);
// register the middleware
app.use('/myCustomAPIEndpoint', Fetcher.middleware());

app.use(function(req, res, next) {
app.use(function (req, res, next) {
// instantiated fetcher with access to req object
const fetcher = new Fetcher({
xhrPath: '/myCustomAPIEndpoint', // xhrPath will be ignored on the serverside fetcher instantiation
req: req
req: req,
});

// perform read call to get data
fetcher
.read('data_service')
.params({id: ###})
.end(function (err, data, meta) {
// handle err and/or data returned from data fetcher in this callback
.params({ id: 42 })
Copy link
Contributor

Choose a reason for hiding this comment

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

This is more of a style comment and less about the code changes, but I've never been a fan of this programming style. All of this code was written before async / await was available. I would prefer if the API were something simpler like:

try { 
  // read(/* service */, /* params */, /* config */);
  const data = await fetcher.read('data_service', { id: 42 });
} catch (e) {
  console.log(e);
}

Maybe this is something we change for a 1.x release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, I agree. I only fixed the docs because there are people at my company requiring a more up to date docs. I've also revisited what we discussed on #275 and this is what I want to tackle next. I also think it's possible to make it backward compatible so we can adjust everything before going to v1 and remove all the other APIs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense to me. BC is ideal but not a blocker if it makes the API better and user friendly.

.then(({ data, meta }) => {
// handle data returned from data fetcher in this callback
})
.catch((err) => {
// handle error
});
});
```
Expand All @@ -127,21 +130,27 @@ app.use(function(req, res, next) {
// app.js - client
import Fetcher from 'fetchr';
const fetcher = new Fetcher({
xhrPath: '/myCustomAPIEndpoint' // xhrPath is REQUIRED on the clientside fetcher instantiation
xhrPath: '/myCustomAPIEndpoint', // xhrPath is REQUIRED on the clientside fetcher instantiation
});
fetcher
.read('data_api_fetcher')
.params({id: ###})
.end(function (err, data, meta) {
// handle err and/or data returned from data fetcher in this callback
.params({ id: 42 })
.then(({ data, meta }) => {
// handle data returned from data fetcher in this callback
})
.catch((err) => {
// handle errors
});

// for create you can use the body() method to pass data
fetcher
.create('data_api_create')
.body({"some":"data"})
.end(function (err, data, meta) {
// handle err and/or data returned from data fetcher in this callback
.body({ some: 'data' })
.then(({ data, meta }) => {
// handle data returned from data fetcher in this callback
})
.catch((err) => {
// handle errors
});
```

Expand Down Expand Up @@ -182,7 +191,7 @@ export default {
fetcher
.read('data_service')
.params({id: ###})
.end(function (err, data, meta) {
.then(({ data, meta }) {
// data will be 'response'
// meta will have the header and statusCode from above
});
Expand Down Expand Up @@ -237,33 +246,31 @@ And in your service call:
fetcher
.read('someData')
.params({ id: '42' })
.end(function (err, data, meta) {
if (err) {
// err instanceof FetchrError -> true
// err.message -> "Not found"
// err.meta -> { foo: 'bar' }
// err.name = 'FetchrError'
// err.output -> { message: "Not found", more: "meta data" }
// err.rawRequest -> { headers: {}, method: 'GET', url: '/api/someData' }
// err.reason -> BAD_HTTP_STATUS | BAD_JSON | TIMEOUT | ABORT | UNKNOWN
// err.statusCode -> 404
// err.timeout -> 3000
// err.url -> '/api/someData'
}
.catch((err) => {
// err instanceof FetchrError -> true
// err.message -> "Not found"
// err.meta -> { foo: 'bar' }
// err.name = 'FetchrError'
// err.output -> { message: "Not found", more: "meta data" }
// err.rawRequest -> { headers: {}, method: 'GET', url: '/api/someData' }
// err.reason -> BAD_HTTP_STATUS | BAD_JSON | TIMEOUT | ABORT | UNKNOWN
// err.statusCode -> 404
// err.timeout -> 3000
// err.url -> '/api/someData'
});
```

## Abort support

An object with an `abort` method is returned by the `.end()` method.
An object with an `abort` method is returned when creating fetchr requests on client.
This is useful if you want to abort a request before it is completed.

```js
const req = fetcher
.read('someData')
.params({id: ###})
.end(function (err, data, meta) {
// err.output will be { message: "Not found", more: "meta data" }
.params({ id: 42 })
.catch((err) => {
// err.reason will be ABORT
});

req.abort();
Expand All @@ -283,21 +290,21 @@ const fetcher = new Fetcher({
});
```

If you have an individual request that you need to ensure has a specific timeout you can do that via the `timeout` option in `clientConfig`:
If you want to set a timeout per request you can call `clientConfig` with a `timeout` property:

```js
fetcher
.read('someData')
.params({id: ###})
.clientConfig({timeout: 5000}) // wait 5 seconds for this request before timing it out
.end(function (err, data, meta) {
// handle err and/or data returned from data fetcher in this callback
.params({ id: 42 })
.clientConfig({ timeout: 5000 }) // wait 5 seconds for this request before timing out
.catch((err) => {
// err.reason will be TIMEOUT
});
```

## Params Processing

For some applications, there may be a situation where you need to process the service params passed in the request before they are sent to the actual service. Typically, you would process them in the service itself. However, if you neet to perform processing across many services (i.e. sanitization for security), then you can use the `paramsProcessor` option.
For some applications, there may be a situation where you need to process the service params passed in the request before they are sent to the actual service. Typically, you would process them in the service itself. However, if you need to perform processing across many services (i.e. sanitization for security), then you can use the `paramsProcessor` option.

`paramsProcessor` is a function that is passed into the `Fetcher.middleware` method. It is passed three arguments, the request object, the serviceInfo object, and the service params object. The `paramsProcessor` function can then modify the service params if needed.

Expand Down Expand Up @@ -357,11 +364,7 @@ const fetcher = new Fetcher({
corsPath: 'http://www.foo.com',
xhrPath: '/fooProxy',
});
fetcher
.read('service')
.params({ foo: 1 })
.clientConfig({ cors: true })
.end(callbackFn);
fetcher.read('service').params({ foo: 1 }).clientConfig({ cors: true });
```

Additionally, you can also customize how the GET URL is constructed by passing in the `constructGetUri` property when you execute your `read` call:
Expand All @@ -381,14 +384,10 @@ const fetcher = new Fetcher({
corsPath: 'http://www.foo.com',
xhrPath: '/fooProxy',
});
fetcher
.read('service')
.params({ foo: 1 })
.clientConfig({
cors: true,
constructGetUri: customConstructGetUri,
})
.end(callbackFn);
fetcher.read('service').params({ foo: 1 }).clientConfig({
cors: true,
constructGetUri: customConstructGetUri,
});
```

## CSRF Protection
Expand Down Expand Up @@ -426,7 +425,7 @@ const config = {
unsafeAllowRetry: false, // for POST requests, whether to allow retrying this post
};

fetcher.read('service').params({ id: 1 }).clientConfig(config).end(callbackFn);
fetcher.read('service').params({ id: 1 }).clientConfig(config);
```

For requests from the server, the config object is simply passed into the service being called.
Expand All @@ -442,12 +441,9 @@ const fetchr = new Fetchr({
});

// Per request
fetchr
.read('service')
.clientConfig({
retry: { maxRetries: 1 },
})
.end();
fetchr.read('service').clientConfig({
retry: { maxRetries: 1 },
});
```

With the above configuration, Fetchr will retry twice all requests
Expand Down Expand Up @@ -544,7 +540,7 @@ const config = {
},
};

fetcher.read('service').params({ id: 1 }).clientConfig(config).end(callbackFn);
fetcher.read('service').params({ id: 1 }).clientConfig(config);
```

All requests contain custom headers when you add `headers` option to constructor arguments of 'Fetcher'.
Expand Down
72 changes: 40 additions & 32 deletions libs/fetcher.client.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,24 @@ Request.prototype._captureMetaAndStats = function (err, result) {
}
};

Request.prototype.then = function (resolve, reject) {
return this.end(function (err, data, meta) {
if (err) {
reject(err);
} else {
resolve({ data, meta });
}
});
};

Request.prototype.catch = function (reject) {
return this.end(function (err) {
if (err) {
reject(err);
}
});
};

/**
* Execute this fetcher request and call callback.
* @method end
Expand All @@ -112,54 +130,44 @@ Request.prototype._captureMetaAndStats = function (err, result) {
* @async
*/
Request.prototype.end = function (callback) {
if (!callback) {
console.warn(
'You called .end() without a callback. This will become an error in the future. Use .then() instead.',
);
}

var self = this;
self._startTime = Date.now();
var request = httpRequest(normalizeOptions(self));

if (callback) {
request.then(
function (result) {
self._captureMetaAndStats(null, result);
setTimeout(function () {
callback(
null,
result && result.data,
result && result.meta,
);
});
},
function (err) {
self._captureMetaAndStats(err);
setTimeout(function () {
callback(err);
});
},
);
return request;
}
var onResponse = function (err, result) {
self._captureMetaAndStats(err, result);
if (callback) {
setTimeout(function () {
callback(err, result && result.data, result && result.meta);
});
} else if (err) {
throw err;
} else {
return result;
}
};

var promise = request.then(
return httpRequest(normalizeOptions(self)).then(
function (result) {
self._captureMetaAndStats(null, result);
return result;
return onResponse(null, result);
},
function (err) {
self._captureMetaAndStats(err);
throw err;
return onResponse(err);
},
);

promise.abort = request.abort;

return promise;
};

/**
* Fetcher class for the client. Provides CRUD methods.
* @class FetcherClient
* @param {Object} options configuration options for Fetcher
* @param {String} [options.xhrPath="/api"] The path for requests
* @param {Number} [options.xhrTimout=3000] Timeout in milliseconds for all requests
* @param {Number} [options.xhrTimeout=3000] Timeout in milliseconds for all requests
* @param {Boolean} [options.corsPath] Base CORS path in case CORS is enabled
* @param {Object} [options.context] The context object that is propagated to all outgoing
* requests as query params. It can contain current-session/context data that should
Expand Down
24 changes: 24 additions & 0 deletions libs/fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,12 @@ class Request {
* is complete.
*/
end(callback) {
if (!callback) {
console.warn(
'You called .end() without a callback. This will become an error in the future. Use .then() instead.',
);
}

this._startTime = Date.now();

const promise = new Promise((resolve, reject) => {
Expand Down Expand Up @@ -254,6 +260,24 @@ class Request {
return promise;
}
}

then(resolve, reject) {
return this.end((err, data, meta) => {
if (err) {
reject(err);
} else {
resolve({ data, meta });
}
});
}

catch(reject) {
return this.end((err) => {
if (err) {
reject(err);
}
});
}
}

/**
Expand Down
Loading
Loading