Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions lib/shared/config-manager/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"extends": ["../../../.eslintrc.json"],
"ignorePatterns": ["!**/*"],
"overrides": [
{
"files": ["*.ts", "*.tsx", "*.js", "*.jsx"],
"rules": {}
},
{
"files": ["*.ts", "*.tsx"],
"rules": {}
},
{
"files": ["*.js", "*.jsx"],
"rules": {}
}
]
}
12 changes: 12 additions & 0 deletions lib/shared/config-manager/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# config-manager

This library extracts the `EnvironmentConfigManager` Server SDK logic to be used across the NodeJS SDK
and Edge Worker SDKs.

## Building

Run `nx build config-manager` to build the library.

## Running unit tests

Run `nx test config-manager` to execute the unit tests via [Jest](https://jestjs.io).
Original file line number Diff line number Diff line change
@@ -1,23 +1,40 @@
jest.mock('../src/request')
jest.useFakeTimers()
jest.spyOn(global, 'setInterval')
jest.mock('../src/bucketing')

import { EnvironmentConfigManager } from '../src/environmentConfigManager'
import { importBucketingLib, getBucketingLib } from '../src/bucketing'
import { EnvironmentConfigManager } from '../src'
import { mocked } from 'jest-mock'
import { Response } from 'cross-fetch'
import { dvcDefaultLogger, ResponseError } from '@devcycle/js-cloud-server-sdk'
import {
DevCycleOptions,
dvcDefaultLogger,
ResponseError,
} from '@devcycle/js-cloud-server-sdk'
import { DVCLogger } from '@devcycle/types'
import { getEnvironmentConfig } from '../src/request'

const setInterval_mock = mocked(setInterval)
const getEnvironmentConfig_mock = mocked(getEnvironmentConfig)
const logger = dvcDefaultLogger()

const mockSDKConfig = jest.fn()

function getConfigManager(
Copy link
Member Author

Choose a reason for hiding this comment

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

Moving test to new config-manager lib

logger: DVCLogger,
sdkKey: string,
options: DevCycleOptions,
) {
return new EnvironmentConfigManager(
logger,
sdkKey,
mockSDKConfig,
setInterval,
clearInterval,
options,
)
}

describe('EnvironmentConfigManager Unit Tests', () => {
beforeAll(async () => {
await importBucketingLib()
})
beforeEach(() => {
getEnvironmentConfig_mock.mockReset()
setInterval_mock.mockReset()
Expand All @@ -43,18 +60,15 @@ describe('EnvironmentConfigManager Unit Tests', () => {
mockFetchResponse({ status: 200 }),
)

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {
const envConfig = getConfigManager(logger, 'sdkKey', {
configPollingIntervalMS: 1000,
configPollingTimeoutMS: 1000,
})
await envConfig.fetchConfigPromise
expect(setInterval_mock).toHaveBeenCalledTimes(1)

await envConfig._fetchConfig()
expect(getBucketingLib().setConfigDataUTF8).toHaveBeenCalledWith(
'sdkKey',
Buffer.from('{}', 'utf8'),
)
expect(mockSDKConfig).toHaveBeenCalledWith('sdkKey', '{}')

expect(envConfig).toEqual(
expect.objectContaining({
Expand All @@ -72,7 +86,7 @@ describe('EnvironmentConfigManager Unit Tests', () => {
mockFetchResponse({ status: 200 }),
)

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {
const envConfig = getConfigManager(logger, 'sdkKey', {
configPollingIntervalMS: 10,
configPollingTimeoutMS: 10000,
})
Expand All @@ -97,7 +111,7 @@ describe('EnvironmentConfigManager Unit Tests', () => {
mockFetchResponse({ status: 200 }),
)

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {
const envConfig = getConfigManager(logger, 'sdkKey', {
configPollingIntervalMS: 1000,
configPollingTimeoutMS: 1000,
})
Expand Down Expand Up @@ -129,7 +143,7 @@ describe('EnvironmentConfigManager Unit Tests', () => {
mockFetchResponse({ status: 500 }),
)

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {})
const envConfig = getConfigManager(logger, 'sdkKey', {})
expect(envConfig.fetchConfigPromise).rejects.toThrow(
'Failed to download DevCycle config.',
)
Expand All @@ -140,7 +154,7 @@ describe('EnvironmentConfigManager Unit Tests', () => {
mockFetchResponse({ status: 403 }),
)

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {})
const envConfig = getConfigManager(logger, 'sdkKey', {})
expect(envConfig.fetchConfigPromise).rejects.toThrow(
'Invalid SDK key provided:',
)
Expand All @@ -150,7 +164,7 @@ describe('EnvironmentConfigManager Unit Tests', () => {
it('should throw error fetching config throws', () => {
getEnvironmentConfig_mock.mockRejectedValue(new Error('Error'))

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {})
const envConfig = getConfigManager(logger, 'sdkKey', {})
expect(envConfig.fetchConfigPromise).rejects.toThrow(
'Failed to download DevCycle config.',
)
Expand All @@ -162,7 +176,7 @@ describe('EnvironmentConfigManager Unit Tests', () => {
mockFetchResponse({ status: 200, data: config }),
)

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {
const envConfig = getConfigManager(logger, 'sdkKey', {
configPollingIntervalMS: 1000,
configPollingTimeoutMS: 1000,
})
Expand All @@ -184,7 +198,7 @@ describe('EnvironmentConfigManager Unit Tests', () => {
mockFetchResponse({ status: 500 }),
)

const envConfig = new EnvironmentConfigManager(logger, 'sdkKey', {})
const envConfig = getConfigManager(logger, 'sdkKey', {})
await expect(envConfig.fetchConfigPromise).rejects.toThrow()
expect(setInterval_mock).toHaveBeenCalledTimes(1)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,9 @@ import fetch, { Response } from 'cross-fetch'

global.fetch = fetch

import { getEnvironmentConfig } from '../src/request'
const fetchRequestMock = fetch as jest.MockedFn<typeof fetch>

import { publishEvents, getEnvironmentConfig } from '../src/request'
import { dvcDefaultLogger } from '@devcycle/js-cloud-server-sdk'
const logger = dvcDefaultLogger()

describe('request.ts Unit Tests', () => {
beforeEach(() => {
fetchRequestMock.mockReset()
Expand Down
13 changes: 13 additions & 0 deletions lib/shared/config-manager/jest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/* eslint-disable */
export default {
displayName: 'config-manager',
preset: '../../../jest.preset.js',
transform: {
'^.+\\.[tj]s$': [
'ts-jest',
{ tsconfig: '<rootDir>/tsconfig.spec.json' },
],
},
moduleFileExtensions: ['ts', 'js', 'html'],
coverageDirectory: '../../../coverage/lib/shared/config-manager',
}
10 changes: 10 additions & 0 deletions lib/shared/config-manager/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "@devcycle/config-manager",
"private": true,
"version": "1.0.0",
"type": "commonjs",
"dependencies": {
"@devcycle/js-cloud-server-sdk": "^1.0.0",
"@devcycle/types": "^1.1.15"
}
}
41 changes: 41 additions & 0 deletions lib/shared/config-manager/project.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{
"name": "config-manager",
"$schema": "../../../node_modules/nx/schemas/project-schema.json",
"sourceRoot": "lib/shared/config-manager/src",
"projectType": "library",
"targets": {
"build": {
"executor": "@nx/js:tsc",
"outputs": ["{options.outputPath}"],
"options": {
"outputPath": "dist/lib/shared/config-manager",
"main": "lib/shared/config-manager/src/index.ts",
"tsConfig": "lib/shared/config-manager/tsconfig.lib.json",
"assets": ["lib/shared/config-manager/*.md"],
"external": ["shared-types", "js-cloud-server-sdk"]
}
},
"lint": {
"executor": "@nx/linter:eslint",
"outputs": ["{options.outputFile}"],
"options": {
"lintFilePatterns": ["lib/shared/config-manager/**/*.ts"]
}
},
"test": {
"executor": "@nx/jest:jest",
"outputs": ["{workspaceRoot}/coverage/{projectRoot}"],
"options": {
"jestConfig": "lib/shared/config-manager/jest.config.ts",
"passWithNoTests": true
},
"configurations": {
"ci": {
"ci": true,
"codeCoverage": true
}
}
}
},
"tags": []
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,25 @@
import { DVCLogger } from '@devcycle/types'
import { getBucketingLib } from './bucketing'
import { UserError } from './utils/userError'
// import { UserError } from './utils/userError'
import { getEnvironmentConfig } from './request'
import { ResponseError, DevCycleOptions } from '@devcycle/js-cloud-server-sdk'

type ConfigPollingOptions = DevCycleOptions & {
cdnURI?: string
}

type SetIntervalInterface = (handler: () => void, timeout?: number) => any
type ClearIntervalInterface = (intervalTimeout: any) => void

type SetConfigBuffer = (sdkKey: string, projectConfig: string) => void

export class UserError extends Error {
constructor(error: Error | string) {
super(error instanceof Error ? error.message : error)
this.name = 'UserError'
this.stack = error instanceof Error ? error.stack : undefined
}
}

export class EnvironmentConfigManager {
private readonly logger: DVCLogger
private readonly sdkKey: string
Expand All @@ -17,12 +29,18 @@ export class EnvironmentConfigManager {
private readonly requestTimeoutMS: number
private readonly cdnURI: string
fetchConfigPromise: Promise<void>
private intervalTimeout?: NodeJS.Timeout
private intervalTimeout?: any
Copy link
Contributor

Choose a reason for hiding this comment

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

why does this need to be any?

Copy link
Member Author

Choose a reason for hiding this comment

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

basically because NodeJS.Timeout isn't supported in webworker / browser environments, they just return a number I believe. So the strategy here was to add a setInterval + clearInterval interface that would be passed in, so this needs to be an Any if we want it to compile when used in the non-NodeJS SDKs.

Copy link
Contributor

Choose a reason for hiding this comment

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

if you wanted to get fancy you could use a type generic for this. Type it so that the generic is what is returned by the SetIntervalInterface and then use the same generic here.

private disablePolling = false
private readonly setConfigBuffer: SetConfigBuffer
private readonly setInterval: SetIntervalInterface
private readonly clearInterval: ClearIntervalInterface

constructor(
logger: DVCLogger,
sdkKey: string,
setConfigBuffer: SetConfigBuffer,
setInterval: SetIntervalInterface,
Copy link
Contributor

Choose a reason for hiding this comment

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

this polling config fetcher seems very specific to the nodejs local bucketing SDK, what purpose would it serve in the worker SDK other than to fetch it once?

Copy link
Member Author

Choose a reason for hiding this comment

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

workers do last for a while (I think I've seen 10's of minutes if not longer), we'd still want polling for them.

clearInterval: ClearIntervalInterface,
{
configPollingIntervalMS = 10000,
configPollingTimeoutMS = 5000,
Expand All @@ -32,6 +50,11 @@ export class EnvironmentConfigManager {
) {
this.logger = logger
this.sdkKey = sdkKey

this.setConfigBuffer = setConfigBuffer
this.setInterval = setInterval
this.clearInterval = clearInterval

this.pollingIntervalMS =
configPollingIntervalMS >= 1000 ? configPollingIntervalMS : 1000
this.requestTimeoutMS =
Expand All @@ -48,19 +71,19 @@ export class EnvironmentConfigManager {
if (this.disablePolling) {
return
}
this.intervalTimeout = setInterval(async () => {
this.intervalTimeout = this.setInterval(async () => {
try {
await this._fetchConfig()
} catch (ex) {
this.logger.error(ex.message)
this.logger.error((ex as Error).message)
}
}, this.pollingIntervalMS)
})
}

stopPolling(): void {
this.disablePolling = true
clearInterval(this.intervalTimeout)
this.clearInterval(this.intervalTimeout)
}

cleanup(): void {
Expand Down Expand Up @@ -119,8 +142,7 @@ export class EnvironmentConfigManager {
} else if (res?.status === 200 && projectConfig) {
try {
const etag = res?.headers.get('etag') || ''
const configBuffer = Buffer.from(projectConfig, 'utf8')
getBucketingLib().setConfigDataUTF8(this.sdkKey, configBuffer)
this.setConfigBuffer(this.sdkKey, projectConfig)
this.hasConfig = true
this.configEtag = etag
return
Expand Down
38 changes: 38 additions & 0 deletions lib/shared/config-manager/src/request.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { RequestInitWithRetry } from 'fetch-retry'
import { get } from '@devcycle/js-cloud-server-sdk'

export async function getEnvironmentConfig(
Copy link
Member Author

Choose a reason for hiding this comment

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

move the request.ts methods only used by the config-manager here.

url: string,
requestTimeout: number,
etag?: string,
): Promise<Response> {
const headers: Record<string, string> = etag
? { 'If-None-Match': etag }
: {}

return await getWithTimeout(
url,
{
headers: headers,
retries: 1,
},
requestTimeout,
)
}

async function getWithTimeout(
url: string,
requestConfig: RequestInit | RequestInitWithRetry,
timeout: number,
): Promise<Response> {
const controller = new AbortController()
const id = setTimeout(() => {
controller.abort()
}, timeout)
const response = await get(url, {
...requestConfig,
signal: controller.signal,
})
clearTimeout(id)
return response
}
13 changes: 13 additions & 0 deletions lib/shared/config-manager/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"extends": "../../../tsconfig.base.json",
"files": [],
"include": [],
"references": [
{
"path": "./tsconfig.lib.json"
},
{
"path": "./tsconfig.spec.json"
}
]
}
Loading