From c0ac8b297781d4bab536a6d59143c612fed26d0f Mon Sep 17 00:00:00 2001 From: Annie Li Date: Tue, 19 Mar 2024 21:08:34 -0700 Subject: [PATCH 1/4] accept hardcoded type in StripeError constructor --- .gitignore | 1 + src/Error.ts | 66 ++++++++++++++++++++++++++++++++-------- test/Error.spec.ts | 13 ++++++-- test/Integration.spec.ts | 30 ++++++++++++++++++ 4 files changed, 95 insertions(+), 15 deletions(-) diff --git a/.gitignore b/.gitignore index b0cb00fb77..100c97bed5 100644 --- a/.gitignore +++ b/.gitignore @@ -7,5 +7,6 @@ tags coverage .idea testProjects/**/node_modules +testProjects/**/dist testProjects/**/package-lock.json testProjects/**/_worker.* diff --git a/src/Error.ts b/src/Error.ts index 75291066fc..d2118557b1 100644 --- a/src/Error.ts +++ b/src/Error.ts @@ -49,9 +49,9 @@ export class StripeError extends Error { readonly setup_intent?: any; readonly source?: any; - constructor(raw: StripeRawError = {}) { + constructor(raw: StripeRawError = {}, type = 'StripeError') { super(raw.message); - this.type = this.constructor.name; + this.type = type; this.raw = raw; this.rawType = raw.type; @@ -86,13 +86,21 @@ export class StripeError extends Error { * CardError is raised when a user enters a card that can't be charged for * some reason. */ -export class StripeCardError extends StripeError {} +export class StripeCardError extends StripeError { + constructor(raw: StripeRawError = {}) { + super(raw, 'StripeCardError'); + } +} /** * InvalidRequestError is raised when a request is initiated with invalid * parameters. */ -export class StripeInvalidRequestError extends StripeError {} +export class StripeInvalidRequestError extends StripeError { + constructor(raw: StripeRawError = {}) { + super(raw, 'StripeInvalidRequestError'); + } +} /** * APIError is a generic error that may be raised in cases where none of the @@ -100,33 +108,53 @@ export class StripeInvalidRequestError extends StripeError {} * that a new error has been introduced in the API, but this version of the * Node.JS SDK doesn't know how to handle it. */ -export class StripeAPIError extends StripeError {} +export class StripeAPIError extends StripeError { + constructor(raw: StripeRawError = {}) { + super(raw, 'StripeAPIError'); + } +} /** * AuthenticationError is raised when invalid credentials are used to connect * to Stripe's servers. */ -export class StripeAuthenticationError extends StripeError {} +export class StripeAuthenticationError extends StripeError { + constructor(raw: StripeRawError = {}) { + super(raw, 'StripeAuthenticationError'); + } +} /** * PermissionError is raised in cases where access was attempted on a resource * that wasn't allowed. */ -export class StripePermissionError extends StripeError {} +export class StripePermissionError extends StripeError { + constructor(raw: StripeRawError = {}) { + super(raw, 'StripePermissionError'); + } +} /** * RateLimitError is raised in cases where an account is putting too much load * on Stripe's API servers (usually by performing too many requests). Please * back off on request rate. */ -export class StripeRateLimitError extends StripeError {} +export class StripeRateLimitError extends StripeError { + constructor(raw: StripeRawError = {}) { + super(raw, 'StripeRateLimitError'); + } +} /** * StripeConnectionError is raised in the event that the SDK can't connect to * Stripe's servers. That can be for a variety of different reasons from a * downed network to a bad TLS certificate. */ -export class StripeConnectionError extends StripeError {} +export class StripeConnectionError extends StripeError { + constructor(raw: StripeRawError = {}) { + super(raw, 'StripeConnectionError'); + } +} /** * SignatureVerificationError is raised when the signature verification for a @@ -141,7 +169,7 @@ export class StripeSignatureVerificationError extends StripeError { payload: string | Uint8Array, raw: StripeRawError = {} ) { - super(raw); + super(raw, 'StripeSignatureVerificationError'); this.header = header; this.payload = payload; } @@ -151,7 +179,11 @@ export class StripeSignatureVerificationError extends StripeError { * IdempotencyError is raised in cases where an idempotency key was used * improperly. */ -export class StripeIdempotencyError extends StripeError {} +export class StripeIdempotencyError extends StripeError { + constructor(raw: StripeRawError = {}) { + super(raw, 'StripeIdempotencyError'); + } +} /** * InvalidGrantError is raised when a specified code doesn't exist, is @@ -159,9 +191,17 @@ export class StripeIdempotencyError extends StripeError {} * exist, or doesn't belong to you; or if an API key's mode (live or test) * doesn't match the mode of a code or refresh token. */ -export class StripeInvalidGrantError extends StripeError {} +export class StripeInvalidGrantError extends StripeError { + constructor(raw: StripeRawError = {}) { + super(raw, 'StripeInvalidGrantError'); + } +} /** * Any other error from Stripe not specifically captured above */ -export class StripeUnknownError extends StripeError {} +export class StripeUnknownError extends StripeError { + constructor(raw: StripeRawError = {}) { + super(raw, 'StripeUnknownError'); + } +} diff --git a/test/Error.spec.ts b/test/Error.spec.ts index 9e6ea65173..bc6db0742c 100644 --- a/test/Error.spec.ts +++ b/test/Error.spec.ts @@ -58,10 +58,19 @@ describe('Error', () => { expect(e).to.have.property('statusCode', 400); }); - it('has subclasses which provide `.type` as their name', () => { - class Foo extends Error.StripeError {} + it('respects subclasses overriding type in constructor', () => { + class Foo extends Error.StripeError { + constructor(raw: any) { + super(raw, 'Foo'); + } + } const err = new Foo({message: 'hi'}); expect(err).to.have.property('type', 'Foo'); }); + + it('StripeError constructor defaults to `type: "StripeError"`', () => { + const err = new Error.StripeError({message: 'hi'}); + expect(err).to.have.property('type', 'StripeError'); + }); }); }); diff --git a/test/Integration.spec.ts b/test/Integration.spec.ts index 1483de9c3a..c505b14715 100644 --- a/test/Integration.spec.ts +++ b/test/Integration.spec.ts @@ -55,6 +55,36 @@ describe('Integration test', function() { await runTestProject('mjs-ts'); }); + describe('esbuild', () => { + it('should build with esbuild', async function() { + // Node supports ES Modules starting at v12 + if (nodeVersion <= 12) { + this.skip(); + } + + await testExec(` + cd testProjects/esbuild && rm -rf node_modules && rm -rf dist + npm install && + npm run build:unminified && + npm run runtestproject -- ${FAKE_API_KEY} + `); + }); + + it('should not change error.type when minified', async function() { + // Node supports ES Modules starting at v12 + if (nodeVersion <= 12) { + this.skip(); + } + + await testExec(` + cd testProjects/esbuild && rm -rf node_modules && rm -rf dist + npm install && + npm run build:minified && + npm run runtestproject -- ${FAKE_API_KEY} + `); + }); + }); + const runTestCloudflareProject = (projectName: string): Promise => { if (process.versions.node < '16.13') { console.log('Wrangler requires at least node.js v16.13.0, skipping test'); From 08ecd3fd9f33582e2ad3243c1c32e9fc498e6872 Mon Sep 17 00:00:00 2001 From: Annie Li Date: Wed, 20 Mar 2024 08:53:30 -0700 Subject: [PATCH 2/4] Default to this.constructor.name when type is not provided --- src/Error.ts | 4 ++-- test/Error.spec.ts | 9 +++++---- test/Integration.spec.ts | 18 ++---------------- 3 files changed, 9 insertions(+), 22 deletions(-) diff --git a/src/Error.ts b/src/Error.ts index d2118557b1..aa094c0952 100644 --- a/src/Error.ts +++ b/src/Error.ts @@ -49,9 +49,9 @@ export class StripeError extends Error { readonly setup_intent?: any; readonly source?: any; - constructor(raw: StripeRawError = {}, type = 'StripeError') { + constructor(raw: StripeRawError = {}, type: string | null = null) { super(raw.message); - this.type = type; + this.type = type || this.constructor.name; this.raw = raw; this.rawType = raw.type; diff --git a/test/Error.spec.ts b/test/Error.spec.ts index bc6db0742c..a899766a7a 100644 --- a/test/Error.spec.ts +++ b/test/Error.spec.ts @@ -58,7 +58,7 @@ describe('Error', () => { expect(e).to.have.property('statusCode', 400); }); - it('respects subclasses overriding type in constructor', () => { + it('respects subclasses overriding `.type` in constructor', () => { class Foo extends Error.StripeError { constructor(raw: any) { super(raw, 'Foo'); @@ -68,9 +68,10 @@ describe('Error', () => { expect(err).to.have.property('type', 'Foo'); }); - it('StripeError constructor defaults to `type: "StripeError"`', () => { - const err = new Error.StripeError({message: 'hi'}); - expect(err).to.have.property('type', 'StripeError'); + it('defaults setting `.type` to subclass name', () => { + class Foo extends Error.StripeError {} + const err = new Foo({message: 'hi'}); + expect(err).to.have.property('type', 'Foo'); }); }); }); diff --git a/test/Integration.spec.ts b/test/Integration.spec.ts index c505b14715..f97f74438e 100644 --- a/test/Integration.spec.ts +++ b/test/Integration.spec.ts @@ -56,20 +56,6 @@ describe('Integration test', function() { }); describe('esbuild', () => { - it('should build with esbuild', async function() { - // Node supports ES Modules starting at v12 - if (nodeVersion <= 12) { - this.skip(); - } - - await testExec(` - cd testProjects/esbuild && rm -rf node_modules && rm -rf dist - npm install && - npm run build:unminified && - npm run runtestproject -- ${FAKE_API_KEY} - `); - }); - it('should not change error.type when minified', async function() { // Node supports ES Modules starting at v12 if (nodeVersion <= 12) { @@ -79,8 +65,8 @@ describe('Integration test', function() { await testExec(` cd testProjects/esbuild && rm -rf node_modules && rm -rf dist npm install && - npm run build:minified && - npm run runtestproject -- ${FAKE_API_KEY} + npm run build && + npm run start -- ${FAKE_API_KEY} `); }); }); From e8a724784ce48373203b2f925f52b6aebf5d1bf4 Mon Sep 17 00:00:00 2001 From: Annie Li Date: Wed, 20 Mar 2024 09:06:12 -0700 Subject: [PATCH 3/4] Add test project --- test/Integration.spec.ts | 2 +- testProjects/esbuild/.eslintrc.cjs | 14 ++++++++++++ testProjects/esbuild/index.js | 35 ++++++++++++++++++++++++++++++ testProjects/esbuild/package.json | 17 +++++++++++++++ 4 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 testProjects/esbuild/.eslintrc.cjs create mode 100644 testProjects/esbuild/index.js create mode 100644 testProjects/esbuild/package.json diff --git a/test/Integration.spec.ts b/test/Integration.spec.ts index f97f74438e..71f2b087cd 100644 --- a/test/Integration.spec.ts +++ b/test/Integration.spec.ts @@ -66,7 +66,7 @@ describe('Integration test', function() { cd testProjects/esbuild && rm -rf node_modules && rm -rf dist npm install && npm run build && - npm run start -- ${FAKE_API_KEY} + npm run start `); }); }); diff --git a/testProjects/esbuild/.eslintrc.cjs b/testProjects/esbuild/.eslintrc.cjs new file mode 100644 index 0000000000..be7cd47486 --- /dev/null +++ b/testProjects/esbuild/.eslintrc.cjs @@ -0,0 +1,14 @@ +module.exports = { + root: true, + env: { + node: true, + es6: true, + }, + extends: [ + 'eslint:recommended', + 'plugin:eslint-plugin-import/recommended' + ], + rules: { + 'no-unused-vars': 'off' + } +}; diff --git a/testProjects/esbuild/index.js b/testProjects/esbuild/index.js new file mode 100644 index 0000000000..3ed3c57da2 --- /dev/null +++ b/testProjects/esbuild/index.js @@ -0,0 +1,35 @@ +import {Stripe} from 'stripe'; +import assert from 'assert'; + +// API key is null to trigger an authentication error +const stripe = new Stripe(null, { + host: process.env.STRIPE_MOCK_HOST || 'localhost', + port: process.env.STRIPE_MOCK_PORT || 12111, + protocol: 'http', +}); + +try { + throw new stripe.errors.StripeUnknownError({ + charge: 'foo', + unknown_prop: 'bar', + }); +} catch (e) { + assert (e instanceof stripe.errors.StripeUnknownError); + assert (e.type === 'StripeUnknownError'); +} + +async function exampleFunction(args) { + try { + await stripe.paymentIntents.create(args); + } catch (e) { + assert (e instanceof stripe.errors.StripeAuthenticationError); + assert (e.type === 'StripeAuthenticationError'); + } +} + +exampleFunction({ + // The required parameter currency is missing + amount: 2000, + confirm: true, + payment_method: 'pm_card_visa', +}); diff --git a/testProjects/esbuild/package.json b/testProjects/esbuild/package.json new file mode 100644 index 0000000000..8c622e1c5c --- /dev/null +++ b/testProjects/esbuild/package.json @@ -0,0 +1,17 @@ +{ + "name": "mjs", + "version": "1.0.0", + "description": "", + "main": "index.js", + "dependencies": { + "stripe": "file:../../" + }, + "scripts": { + "clean": "rm -rf node_modules && rm -rf dist", + "build": "esbuild index.js --bundle --platform=node --minify --target=es2020 --outdir=dist", + "start": "node dist/index.js" + }, + "devDependencies": { + "esbuild": "^0.20.2" + } +} From 96b2a5e2169e354b55fddcc4f351b9decbcc4c96 Mon Sep 17 00:00:00 2001 From: Annie Li Date: Wed, 20 Mar 2024 09:14:36 -0700 Subject: [PATCH 4/4] Don't need eslint config --- testProjects/esbuild/.eslintrc.cjs | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 testProjects/esbuild/.eslintrc.cjs diff --git a/testProjects/esbuild/.eslintrc.cjs b/testProjects/esbuild/.eslintrc.cjs deleted file mode 100644 index be7cd47486..0000000000 --- a/testProjects/esbuild/.eslintrc.cjs +++ /dev/null @@ -1,14 +0,0 @@ -module.exports = { - root: true, - env: { - node: true, - es6: true, - }, - extends: [ - 'eslint:recommended', - 'plugin:eslint-plugin-import/recommended' - ], - rules: { - 'no-unused-vars': 'off' - } -};