From 1d57e726fd31ef4876db7d98c39b54f3b3e823ac Mon Sep 17 00:00:00 2001 From: Shuai Tseng Date: Mon, 6 Jul 2026 16:21:24 -0700 Subject: [PATCH] Fix "X is not a constructor" TypeError on API error responses lib/error.js defined ValidationError, PermissionError, HttpError, RateLimitError, and ServiceError as arrow functions, but lib/Methods.js and lib/onfleet.js throw them with `new`. Arrow functions have no [[Construct]] behavior, so `new HttpError(...)` (and friends) throws "HttpError is not a constructor" instead of the intended error, swallowing the real Onfleet API error details in the process. Switch the factory functions to `function` declarations, which are constructible, while keeping their existing call signature and the returned Error shape (name/status/cause/request) unchanged. Add regression tests covering all five error branches in Methods.js (RateLimitError, PermissionError, ServiceError x2, HttpError) plus the ValidationError path in the Onfleet constructor, none of which had any test coverage before. --- lib/error.js | 26 +++++++++++---- test/test.js | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 6 deletions(-) diff --git a/lib/error.js b/lib/error.js index cd5f8e4..1a6d054 100644 --- a/lib/error.js +++ b/lib/error.js @@ -8,16 +8,30 @@ const createError = (name, message, status = null, cause = null, request = null) return error; }; -// Create specific error types using the factory function -const ValidationError = (message) => createError('ValidationError', message); +// Create specific error types using the factory function. +// These must be `function` declarations, not arrow functions: they are +// invoked with `new` (see Methods.js and onfleet.js), and arrow functions +// have no [[Construct]] behavior, so `new` on one throws +// "X is not a constructor". +function ValidationError(message) { + return createError('ValidationError', message); +} -const PermissionError = (message, status, cause, request) => createError('PermissionError', message, status, cause, request); +function PermissionError(message, status, cause, request) { + return createError('PermissionError', message, status, cause, request); +} -const HttpError = (message, status, cause, request) => createError('HttpError', message, status, cause, request); +function HttpError(message, status, cause, request) { + return createError('HttpError', message, status, cause, request); +} -const RateLimitError = (message, status, cause, request) => createError('RateLimitError', message, status, cause, request); +function RateLimitError(message, status, cause, request) { + return createError('RateLimitError', message, status, cause, request); +} -const ServiceError = (message, status, cause, request) => createError('ServiceError', message, status, cause, request); +function ServiceError(message, status, cause, request) { + return createError('ServiceError', message, status, cause, request); +} // Export the custom error creators export { diff --git a/test/test.js b/test/test.js index 7331596..1c9fc11 100644 --- a/test/test.js +++ b/test/test.js @@ -471,3 +471,94 @@ describe('Additional resource coverage', () => { assert.equal(res, 200); })); }); + +describe('Error handling testing', () => { + const onfleet = new Onfleet(apiKey); + + afterEach(() => { + nock.cleanAll(); + }); + + const errorBody = (error, message = 'Something went wrong') => ({ + message: { + message, + error, + cause: null, + request: null, + }, + }); + + it('ValidationError - constructing without an API key', () => { + expect(() => new Onfleet()).to.throw(Error).with.property('name', 'ValidationError'); + }); + + it('RateLimitError - error code 2300 raises a RateLimitError, not a TypeError', () => { + nock(baseUrl) + .post((uri) => uri.includes('tasks')) + .reply(429, errorBody(2300, 'Rate limit exceeded')); + + return expect(onfleet.tasks.create({})) + .to.be.rejected + .then((error) => { + expect(error).to.not.be.instanceOf(TypeError); + assert.equal(error.name, 'RateLimitError'); + assert.equal(error.message, 'Rate limit exceeded'); + }); + }); + + it('PermissionError - error code in the 1100-1108 range raises a PermissionError', () => { + nock(baseUrl) + .post((uri) => uri.includes('tasks')) + .reply(403, errorBody(1104, 'Insufficient permissions')); + + return expect(onfleet.tasks.create({})) + .to.be.rejected + .then((error) => { + expect(error).to.not.be.instanceOf(TypeError); + assert.equal(error.name, 'PermissionError'); + assert.equal(error.message, 'Insufficient permissions'); + }); + }); + + it('ServiceError - error code >= 2500 raises a ServiceError', () => { + nock(baseUrl) + .post((uri) => uri.includes('tasks')) + .reply(500, errorBody(2500, 'Internal service error')); + + return expect(onfleet.tasks.create({})) + .to.be.rejected + .then((error) => { + expect(error).to.not.be.instanceOf(TypeError); + assert.equal(error.name, 'ServiceError'); + assert.equal(error.message, 'Internal service error'); + }); + }); + + it('ServiceError - error code 2218 (Auto-Dispatch precondition) raises a ServiceError', () => { + nock(baseUrl) + .post((uri) => uri.includes('tasks')) + .reply(412, errorBody(2218, 'Auto-Dispatch precondition failed')); + + return expect(onfleet.tasks.create({})) + .to.be.rejected + .then((error) => { + expect(error).to.not.be.instanceOf(TypeError); + assert.equal(error.name, 'ServiceError'); + assert.equal(error.message, 'Auto-Dispatch precondition failed'); + }); + }); + + it('HttpError - any other error code raises a generic HttpError', () => { + nock(baseUrl) + .post((uri) => uri.includes('tasks')) + .reply(400, errorBody(1000, 'Bad request')); + + return expect(onfleet.tasks.create({})) + .to.be.rejected + .then((error) => { + expect(error).to.not.be.instanceOf(TypeError); + assert.equal(error.name, 'HttpError'); + assert.equal(error.message, 'Bad request'); + }); + }); +});