Merge pull request #337 from camunda-community-hub/336-OAuth-token-ex…

…piry

fix Wrong expiry timer in OAuthProvider.ts #336

.github/workflows/test.yml

@@ -18,7 +18,7 @@ jobs:
         with:
           node-version: 16.17.0
       - run: npm install
-      - run: npm run test:integration
+      - run: npm run test:docker 
     # env:
     #   ZEEBE_ADDRESS: ${{ secrets.ZEEBE_ADDRESS }}
     #   ZEEBE_CLIENT_ID: ${{ secrets.ZEEBE_CLIENT_ID }}

CHANGELOG.md

@@ -10,7 +10,14 @@ _Changes in APIs or behaviour that may affect existing applications that use zee
 
 _New shiny stuff._
 
--   Camunda Platform 8.3.0 introduces multi-tenancy. To support this, the Node.js client adds an optional `tenantId` parameter to `DeployResource`, `DeployProcess`, `CreateProcessInstance`, `CreateProcessInstanceWithResult`, and `PublishMessage`. You can also specify a `tenantId` in the ZBClient constructor or via the environment variable `ZEEBE_TENANT_ID`. This will be transparently added to all method invocations. See [#330](https://github.com/camunda-community-hub/zeebe-client-node-js/issues/330) for more details.
+-   Camunda Platform 8.3.0 introduces multi-tenancy. To support this, the Node.js client adds an optional `tenantId` parameter to `DeployResource`, `DeployProcess`, `CreateProcessInstance`, `CreateProcessInstanceWithResult`, and `PublishMessage`. You can also specify a `tenantId` in the ZBClient constructor or via the environment variable `ZEEBE_TENANT_ID`. In the case that you specify it via the environment or constructor, it will be transparently added to all method invocations. See [#330](https://github.com/camunda-community-hub/zeebe-client-node-js/issues/330) for more details.
+
+## Fixes
+
+_Things that were broken and are now fixed._
+
+-   An error message "Grpc Stream Error: 16 UNAUTHENTICATED: Failed to parse bearer token, see cause for details" would be logged intermittently. This was because under particular conditions an expired token cached on disk could be used for API calls. To prevent this, the disk-cached token is evicted at the same time as the in-memory token. See [#336](https://github.com/camunda-community-hub/zeebe-client-node-js/issues/336) for more details.
+-   The `onReady` and `onConnection` event tests now pass for Camunda SaaS and Self-Managed started with docker-compose, so these events should be usable in these scenarios. These events do not work against a broker running in Docker. YMMV. See [#215](https://github.com/camunda-community-hub/zeebe-client-node-js/issues/215) for more details.
 
 # Version 8.2.5
 
@@ -26,8 +33,6 @@ _Things that shouldn't have a visible impact._
 
 -   Unit tests used a unique process model for each test run. As a result, the number of deployed process models in a cluster increased over time until a SaaS cluster would fail due to sharding of the ElasticSearch. Unit tests have been refactored to reuse process models. This will have no impact for end-users, but for developers it means that you can use the same cluster for unit tests.
 
-
-
 # Version 8.2.4
 
 ## Fixes

package.json

@@ -1,6 +1,6 @@
 {
 	"name": "zeebe-node",
-	"version": "8.3.0-alpha7",
+	"version": "8.3.0-alpha10",
 	"description": "The Node.js client library for the Zeebe Workflow Automation Engine.",
 	"keywords": [
 		"zeebe",
@@ -41,6 +41,7 @@
 		"test": "jest --detectOpenHandles --testPathIgnorePatterns integration local-integration disconnection",
 		"test:integration": "jest --runInBand --testPathIgnorePatterns disconnection --detectOpenHandles --verbose true",
 		"test:local": "jest --runInBand --verbose true --detectOpenHandles local-integration",
+		"test:docker": "jest --runInBand --testPathIgnorePatterns disconnection local-integration --detectOpenHandles --verbose true",
 		"test:disconnect": "jest --runInBand --verbose true --detectOpenHandles disconnection",
 		"test&docs": "npm test && npm run docs",
 		"dev": "tsc-watch --onSuccess \"npm run test&docs\" -p tsconfig.json --outDir dist"

src/__tests__/OAuthProvider.spec.ts

@@ -197,7 +197,7 @@ test('In-memory cache is populated and evicted after timeout', done => {
 
 				req.on('end', () => {
 					res.writeHead(200, { 'Content-Type': 'application/json' })
-					let expires_in = 2
+					let expires_in = 2 // seconds
 					res.end(
 						'{"access_token": "something", "expires_in": ' +
 							expires_in +

src/__tests__/integration/Worker-onReady.spec.ts

@@ -3,8 +3,7 @@ import { ZBClient } from '../..'
 jest.setTimeout(40000)
 process.env.ZEEBE_NODE_LOG_LEVEL = process.env.ZEEBE_NODE_LOG_LEVEL || 'NONE'
 
-// Currently broken. See #215
-xtest(`Worker calls the onReady handler once if there is a broker`, done => {
+test(`Worker calls the onReady handler once if there is a broker`, done => {
 	let called = 0
 	const zbc2 = new ZBClient()
 	zbc2.createWorker({
@@ -74,7 +73,7 @@ test(`Does not call the onReady handler if there is no broker`, done => {
 	}, 5000)
 })
 
-xtest(`Does not emit the ready event if there is no broker`, done => {
+test(`Does not emit the ready event if there is no broker`, done => {
 	let called = 0
 	const zbc2 = new ZBClient('nobroker')
 	zbc2.createWorker({

src/__tests__/local-integration/OnConnectionError.spec.ts

@@ -19,8 +19,7 @@ test(`Calls the onConnectionError handler if there is no broker and eagerConnect
 		}, 5000)
 	}))
 
-// Currently broken. See #215
-xtest(`Does not call the onConnectionError handler if there is a broker`, () =>
+test(`Does not call the onConnectionError handler if there is a broker`, () =>
 	new Promise(done => {
 		let calledB = 0
 		const zbc2 = new ZBClient({
@@ -44,8 +43,7 @@ xtest(`Does not call the onConnectionError handler if there is a broker`, () =>
 		}, 5000)
 	}))
 
-// Currently broken. See #215
-xtest(`Calls ZBClient onConnectionError once when there is no broker, eagerConnection:true, and workers with no handler`, () =>
+test(`Calls ZBClient onConnectionError once when there is no broker, eagerConnection:true, and workers with no handler`, () =>
 	new Promise(done => {
 		let calledC = 0
 		const zbc2 = new ZBClient('localtoast:234532534', {
@@ -125,8 +123,7 @@ test(`Trailing parameter worker onConnectionError handler API works`, () =>
 		}, 10000)
 	}))
 
-// Currently broken. See #215
-xtest(`Does not call the onConnectionError handler if there is a business error`, () =>
+test(`Does not call the onConnectionError handler if there is a business error`, () =>
 	new Promise(async done => {
 		let calledF = 0
 		let wf = 'arstsrasrateiuhrastulyharsntharsie'

src/lib/OAuthProvider.ts

@@ -251,8 +251,19 @@ export class OAuthProvider {
 			delete this.tokenCache[this.clientId]
 			return
 		}
-		this.expiryTimer = setTimeout(() => delete this.tokenCache[this.clientId], validityPeriod)
-		trace(`${this.uuid} start`)
+		// renew token 1s before it expires to avoid race conditions on the wire
+		// evict disk cache at same time as in-memory cache
+		// See: https://github.com/camunda-community-hub/zeebe-client-node-js/issues/336
+		const minimumCacheLifetime = 0; // Minimum cache lifetime in milliseconds
+		const renewTokenAfterMs = Math.max(validityPeriod - 1000, minimumCacheLifetime)
+		this.expiryTimer = setTimeout(() => {
+			trace(`${this.uuid} token expired`)
+			delete this.tokenCache[this.clientId]
+			if (this.useFileCache && fs.existsSync(this.cachedTokenFile(this.clientId))) {
+				fs.unlinkSync(this.cachedTokenFile(this.clientId))
+			}
+		}, renewTokenAfterMs)
+		trace(`${this.uuid} token expiry timer start: ${renewTokenAfterMs}ms`)
 	}
 
 	private cachedTokenFile = (clientId: string) =>

src/zb/ZBClient.ts

@@ -652,22 +652,18 @@ export class ZBClient extends TypedEmitter<typeof ConnectionStatusEvent> {
 	 * ```
 	 */
 	public createProcessInstance<Variables extends ZB.JSONDoc = ZB.IProcessVariables>(config:ZB.CreateProcessInstanceReq<Variables>): Promise<Grpc.CreateProcessInstanceResponse> {
-		if (!!config.tenantId) {
-			this.logger.logInfo('Multi-tenancy is not yet implemented. The tenantId parameter is provided for development purposes.')
-		}
-
 		const request: ZB.CreateProcessInstanceReq<Variables> = {
 			bpmnProcessId: config.bpmnProcessId,
 			variables: config.variables,
 			version: config.version || -1,
-			startInstructions: config.startInstructions || []
+			startInstructions: config.startInstructions || [],
 		}
 
 
 		const createProcessInstanceRequest: Grpc.CreateProcessInstanceRequest = stringifyVariables({
 			...request,
 			startInstructions: request.startInstructions!,
-			tenantId: this.tenantId
+			tenantId: config.tenantId ?? this.tenantId
 		})
 
 		return this.executeOperation('createProcessInstance', () =>
@@ -699,10 +695,6 @@ export class ZBClient extends TypedEmitter<typeof ConnectionStatusEvent> {
 		config: ZB.CreateProcessInstanceWithResultReq<Variables>
 	): Promise<Grpc.CreateProcessInstanceWithResultResponse<Result>>
 	{
-		if (!!config.tenantId) {
-			this.logger.logInfo('Multi-tenancy is not yet implemented. The tenantId parameter is provided for development purposes.')
-		}
-
 		const request = {
 			bpmnProcessId: config.bpmnProcessId,
 			fetchVariables: config.fetchVariables,
@@ -716,7 +708,7 @@ export class ZBClient extends TypedEmitter<typeof ConnectionStatusEvent> {
 				bpmnProcessId: request.bpmnProcessId,
 				variables: request.variables,
 				version: request.version,
-				tenantId: request.tenantId
+				tenantId: request.tenantId ?? this.tenantId
 			}
 		)
 
@@ -779,9 +771,6 @@ export class ZBClient extends TypedEmitter<typeof ConnectionStatusEvent> {
 			| Grpc.DecisionRequirementsDeployment
 		>
 	> {
-		if (!!resource.tenantId) {
-			this.logger.logInfo('Multi-tenancy is not yet implemented. The tenantId parameter is provided for development purposes.')
-		}
 		const isProcess = (
 			maybeProcess: any
 		): maybeProcess is { process: Buffer; name: string } =>