Skip to content

Commit

Permalink
fix: prevent fetchr to throw a 500 whenever a resource does not imple…
Browse files Browse the repository at this point in the history 
…ment an operation (#546)
  • Loading branch information
@pablopalacios
pablopalacios authored Nov 22, 2024
1 parent 6ea25dc commit 4024cab
Show file tree
Hide file tree
Showing 6 changed files with 271 additions and 134 deletions.
4 changes: 3 additions & 1 deletion libs/fetcher.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,10 @@ class Request {

if (!handler) {
const err = new FetchrError(
`operation: ${this.operation} is undefined on service: ${this.resource}`,
`Operation "${this.operation}" is not allowed for resource "${sanitizeResourceName(this.resource)}"`,
);
err.statusCode = 405;

return { err };
}

Expand Down
4 changes: 3 additions & 1 deletion tests/functional/app.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ app.use(express.json());

app.use('/static', express.static(path.join(__dirname, 'static')));

app.use('/api', Fetchr.middleware());
app.use('/api', Fetchr.middleware(), (err, req, res, next) => {
res.status(err.statusCode || 500).json({ message: err.message });
});

app.get('/', (req, res) => {
res.send(`
Expand Down
28 changes: 28 additions & 0 deletions tests/functional/fetchr.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,34 @@ describe('client/server integration', () => {
});
});
});

describe('client errors', () => {
it('handles unknown resources', async () => {
const response = await page.evaluate(() => {
const fetcher = new Fetchr();
return fetcher
.create('unknown-resource')
.catch((err) => err);
});

expect(response.statusCode).to.equal(400);
expect(response.message).to.equal(
'Resource "unknown*resource" is not registered',
);
});

it('handles not implemented operations', async () => {
const response = await page.evaluate(() => {
const fetcher = new Fetchr();
return fetcher.delete('error').catch((err) => err);
});

expect(response.statusCode).to.equal(405);
expect(JSON.parse(response.message).output.message).to.equal(
'Operation "delete" is not allowed for resource "error"',
);
});
});
});

describe('headers', () => {
Expand Down
153 changes: 109 additions & 44 deletions tests/unit/libs/fetcher.client.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,20 @@ describe('Client Fetcher', function () {
}
};
});

beforeEach(function () {
stats = null;
});
testCrud(params, body, config, callbackWithStats, resolve, reject);

testCrud({
params,
body,
config,
callback: callbackWithStats,
reject,
resolve,
});

after(function () {
validateRequest = null;
});
Expand All @@ -147,15 +157,17 @@ describe('Client Fetcher', function () {
corsPath: corsPath,
});
});

testCrud({
params: params,
body: body,
params,
body,
config: { cors: true },
callback,
reject,
resolve,
disableNoConfigTests: true,
callback: callback,
resolve: resolve,
reject: reject,
});

after(function () {
validateRequest = null;
});
Expand Down Expand Up @@ -222,7 +234,16 @@ describe('Client Fetcher', function () {
xhrTimeout: 4000,
});
});
testCrud(params, body, config, callback, resolve, reject);

testCrud({
params,
body,
config,
callback,
resolve,
reject,
});

after(function () {
mockery.deregisterMock('./util/httpRequest');
mockery.disable();
Expand All @@ -245,17 +266,17 @@ describe('Client Fetcher', function () {
xhrTimeout: 4000,
});
});

testCrud({
params: params,
body: body,
config: {
timeout: 5000,
},
disableNoConfigTests: true,
params,
body,
config: { timeout: 5000 },
callback: callback,
resolve: resolve,
reject: reject,
disableNoConfigTests: true,
});

after(function () {
mockery.deregisterMock('./util/httpRequest');
mockery.disable();
Expand All @@ -277,7 +298,16 @@ describe('Client Fetcher', function () {
context: context,
});
});
testCrud(params, body, config, callback, resolve, reject);

testCrud({
params,
body,
config,
callback,
resolve,
reject,
});

after(function () {
mockery.deregisterMock('./util/httpRequest');
mockery.disable();
Expand Down Expand Up @@ -325,7 +355,14 @@ describe('Client Fetcher', function () {
});
});

testCrud(params, body, config, callback, resolve, reject);
testCrud({
params,
body,
config,
callback,
resolve,
reject,
});
});

describe('Property Name', function () {
Expand All @@ -338,7 +375,14 @@ describe('Client Fetcher', function () {
});
});

testCrud(params, body, config, callback, resolve, reject);
testCrud({
params,
body,
config,
callback,
resolve,
reject,
});
});

describe('Property Names', function () {
Expand All @@ -351,7 +395,14 @@ describe('Client Fetcher', function () {
});
});

testCrud(params, body, config, callback, resolve, reject);
testCrud({
params,
body,
config,
callback,
resolve,
reject,
});
});
});

Expand Down Expand Up @@ -396,7 +447,16 @@ describe('Client Fetcher', function () {
},
});
});
testCrud(params, body, config, callback, resolve, reject);

testCrud({
params,
body,
config,
callback,
resolve,
reject,
});

after(function () {
mockery.deregisterMock('./util/httpRequest');
mockery.disable();
Expand All @@ -418,20 +478,17 @@ describe('Client Fetcher', function () {
context: context,
});
});
var customConfig = {
headers: {
'X-APP-VERSION': VERSION,
},
};

testCrud({
params,
body,
config: { headers: { 'X-APP-VERSION': VERSION } },
callback,
resolve,
reject,
disableNoConfigTests: true,
params: params,
body: body,
config: customConfig,
callback: callback,
resolve: resolve,
reject: reject,
});

after(function () {
mockery.deregisterMock('./util/httpRequest');
mockery.disable();
Expand Down Expand Up @@ -679,7 +736,14 @@ describe('Client Fetcher', function () {
});
});

testCrud(params, body, config, callback, resolve, reject);
testCrud({
params,
body,
config,
callback,
resolve,
reject,
});

after(function () {
mockery.deregisterMock('./util/httpRequest');
Expand All @@ -705,23 +769,24 @@ describe('Client Fetcher', function () {
Fetcher = require('../../../libs/fetcher.client');
this.fetcher = new Fetcher({});
});
var customConfig = {
retry: {
interval: 350,
maxRetries: 2,
statusCodes: [0, 502, 504],
},
unsafeAllowRetry: true,
};

testCrud({
params,
body,
config: {
retry: {
interval: 350,
maxRetries: 2,
statusCodes: [0, 502, 504],
},
unsafeAllowRetry: true,
},
callback,
resolve,
reject,
disableNoConfigTests: true,
params: params,
body: body,
config: customConfig,
callback: callback,
resolve: resolve,
reject: reject,
});

after(function () {
mockery.deregisterMock('./util/httpRequest');
mockery.disable();
Expand Down
12 changes: 10 additions & 2 deletions tests/unit/libs/fetcher.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1293,8 +1293,16 @@ describe('Server Fetcher', function () {
beforeEach(function () {
stats = null;
});
// CRUD
testCrud(params, body, config, callbackWithStats, resolve, reject);

testCrud({
params,
body,
config,
callback: callbackWithStats,
resolve,
reject,
isServer: true,
});
});
});

Expand Down
Loading

0 comments on commit 4024cab

Please sign in to comment.