Skip to content
Draft
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
14 changes: 14 additions & 0 deletions __mocks__/@vtex/diagnostics-semconv.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Mock para @vtex/diagnostics-semconv
const ATTR_VTEX_ACCOUNT_NAME = 'vtex.account.name'
const ATTR_VTEX_IO_WORKSPACE_NAME = 'vtex_io.workspace.name'
const ATTR_VTEX_IO_WORKSPACE_TYPE = 'vtex_io.workspace.type'
const ATTR_VTEX_IO_APP_ID = 'vtex_io.app.id'
const ATTR_VTEX_IO_APP_AUTHOR_TYPE = 'vtex_io.app.author-type'

export {
ATTR_VTEX_ACCOUNT_NAME,
ATTR_VTEX_IO_WORKSPACE_NAME,
ATTR_VTEX_IO_WORKSPACE_TYPE,
ATTR_VTEX_IO_APP_ID,
ATTR_VTEX_IO_APP_AUTHOR_TYPE,
}
22 changes: 19 additions & 3 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,24 @@
module.exports = {
roots: ['<rootDir>/src'],
transform: {
'^.+\\.tsx?$': 'ts-jest',
'^.+\\.tsx?$': ['ts-jest', {
useESM: true,
}],
},
testRegex: '(.*(test|spec)).tsx?$',
testMatch: [
'**/__tests__/**/*.(js|ts)?(x)',
'**/?(*.)+(spec|test).(js|ts)?(x)'
],
testPathIgnorePatterns: [
'.*Test[A-Z].*\\.ts$'
],
testEnvironment: 'node',
}
moduleNameMapper: {
'^@vtex/diagnostics-semconv$': '<rootDir>/__mocks__/@vtex/diagnostics-semconv.ts',
},
moduleFileExtensions: [
'ts',
'js'
],
extensionsToTreatAsEsm: ['.ts'],
}
20 changes: 4 additions & 16 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"scripts": {
"clean": "rimraf lib/",
"watch": "yarn clean && tsc --watch",
"build": "yarn clean && tsc",
"build": "yarn lint && yarn clean && tsc",
"gen": "typescript-json-schema src/responses.ts PublicAppManifest > gen/manifest.schema",
"test": "jest",
"lint": "tslint -c tslint.json './src/**/*.ts'",
Expand All @@ -22,19 +22,6 @@
"ci:test": "yarn test --ci",
"ci:prettier-check": "prettier --check --config ./.prettierrc \"src/**/*.ts\" \"src/**/*.js\""
},
"jest": {
"transform": {
"^.+\\.tsx?$": "ts-jest"
},
"testMatch": [
"**/__tests__/**/*.(js|ts)?(x)",
"**/?(*.)+(spec|test).(js|ts)?(x)"
],
"moduleFileExtensions": [
"ts",
"js"
]
},
"engines": {
"node": ">=8"
},
Expand All @@ -54,6 +41,7 @@
"@types/koa": "^2.11.0",
"@types/koa-compose": "^3.2.3",
"@vtex/diagnostics-nodejs": "0.1.0-io-beta.20",
"@vtex/diagnostics-semconv": "^1.1.2",
"@vtex/node-error-report": "^0.0.3",
"@wry/equality": "^0.1.9",
"agentkeepalive": "^4.0.2",
Expand Down Expand Up @@ -117,11 +105,11 @@
"@types/semver": "^5.5.0",
"@types/uuid": "^3.4.6",
"get-port": "^5.1.1",
"jest": "^25.1.0",
"jest": "^29.7.0",
"npm-run-all": "^4.1.3",
"rimraf": "^2.5.2",
"semaphore-async-await": "^1.5.1",
"ts-jest": "^25.2.1",
"ts-jest": "^29.1.0",
"tslint": "^5.14.0",
"tslint-config-vtex": "^2.1.0",
"tslint-eslint-rules": "^5.4.0",
Expand Down
33 changes: 13 additions & 20 deletions src/HttpClient/HttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,9 @@ import { createHash } from 'crypto'
import { IncomingMessage } from 'http'
import compose from 'koa-compose'
import pLimit from 'p-limit'

import {
BINDING_HEADER,
BODY_HASH,
FORWARDED_HOST_HEADER,
LOCALE_HEADER,
PRODUCT_HEADER,
SEGMENT_HEADER,
SESSION_HEADER,
TENANT_HEADER,
HeaderKeys,
} from '../constants'
import { Logger } from '../service/logger'
import { IOContext } from '../service/worker/runtime/typings'
Expand Down Expand Up @@ -89,14 +82,14 @@ export class HttpClient {
...defaultHeaders,
'Accept-Encoding': 'gzip',
'User-Agent': userAgent,
...host ? { [FORWARDED_HOST_HEADER]: host } : null,
...tenant ? { [TENANT_HEADER]: formatTenantHeaderValue(tenant) } : null,
...binding ? { [BINDING_HEADER]: formatBindingHeaderValue(binding) } : null,
...locale ? { [LOCALE_HEADER]: locale } : null,
...operationId ? { 'x-vtex-operation-id': operationId } : null,
...product ? { [PRODUCT_HEADER]: product } : null,
...segmentToken ? { [SEGMENT_HEADER]: segmentToken } : null,
...sessionToken ? { [SESSION_HEADER]: sessionToken } : null,
...host ? { [HeaderKeys.FORWARDED_HOST]: host } : null,
...tenant ? { [HeaderKeys.TENANT]: formatTenantHeaderValue(tenant) } : null,
...binding ? { [HeaderKeys.BINDING]: formatBindingHeaderValue(binding) } : null,
...locale ? { [HeaderKeys.LOCALE]: locale } : null,
...operationId ? { [HeaderKeys.OPERATION_ID]: operationId } : null,
...product ? { [HeaderKeys.PRODUCT]: product } : null,
...segmentToken ? { [HeaderKeys.SEGMENT]: segmentToken } : null,
...sessionToken ? { [HeaderKeys.SESSION]: sessionToken } : null,
}

if (authType && authToken) {
Expand Down Expand Up @@ -139,16 +132,16 @@ export class HttpClient {
return typeof v !== 'object' || v === null || Array.isArray(v) ? v :
Object.fromEntries(Object.entries(v).sort(([ka], [kb]) =>
ka < kb ? -1 : ka > kb ? 1 : 0))
}
catch(error) {
}
catch(error) {
// I don't believe this will ever happen, but just in case
// Also, I didn't include error as I am unsure if it would have sensitive information
this.logger.warn({message: 'Error while sorting object for cache key'})
return v
}
}


const bodyHash = createHash('md5').update(JSON.stringify(data, deterministicReplacer)).digest('hex')
const cacheableConfig = this.getConfig(url, {
...config,
Expand Down
13 changes: 7 additions & 6 deletions src/HttpClient/middlewares/cache.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/* tslint:disable:no-unnecessary-initializer */
import { AxiosRequestConfig, AxiosResponse } from 'axios'
import { Span } from 'opentracing'

import { CacheLayer } from '../../caches/CacheLayer'
import { LOCALE_HEADER, SEGMENT_HEADER, SESSION_HEADER } from '../../constants'
import { HeaderKeys } from '../../constants'
import { IOContext } from '../../service/worker/runtime/typings'
import { ErrorReport } from '../../tracing'
import { HttpLogEvents } from '../../tracing/LogEvents'
Expand All @@ -15,7 +16,7 @@ const cacheableStatusCodes = [200, 203, 204, 206, 300, 301, 404, 405, 410, 414,

export const cacheKey = (config: AxiosRequestConfig) => {
const {baseURL = '', url = '', params, headers} = config
const locale = headers?.[LOCALE_HEADER]
const locale = headers?.[HeaderKeys.LOCALE]

const encodedBaseURL = baseURL.replace(/\//g, '\\')
const encodedURL = url.replace(/\//g, '\\')
Expand Down Expand Up @@ -97,7 +98,7 @@ export const cacheMiddleware = ({ type, storage, asyncSet }: CacheOptions) => {
const { rootSpan: span, tracer, logger } = ctx.tracing ?? {}

const key = cacheKey(ctx.config)
const segmentToken = ctx.config.headers?.[SEGMENT_HEADER]
const segmentToken = ctx.config.headers?.[HeaderKeys.SEGMENT]
const keyWithSegment = key + segmentToken

span?.log({
Expand Down Expand Up @@ -155,7 +156,7 @@ export const cacheMiddleware = ({ type, storage, asyncSet }: CacheOptions) => {
if (cachedEtag && validateStatus(response.status as number)) {
ctx.config.headers = {
...ctx.config.headers,
'if-none-match': cachedEtag
'if-none-match': cachedEtag,
}
ctx.config.validateStatus = validateStatus
}
Expand Down Expand Up @@ -204,11 +205,11 @@ export const cacheMiddleware = ({ type, storage, asyncSet }: CacheOptions) => {
}

const shouldCache = maxAge || etag
const varySession = ctx.response.headers.vary && ctx.response.headers.vary.includes(SESSION_HEADER)
const varySession = ctx.response.headers.vary && ctx.response.headers.vary.includes(HeaderKeys.SESSION)
if (shouldCache && !varySession) {
const {responseType, responseEncoding: configResponseEncoding} = ctx.config
const currentAge = revalidated ? 0 : age
const varySegment = ctx.response.headers.vary && ctx.response.headers.vary.includes(SEGMENT_HEADER)
const varySegment = ctx.response.headers.vary && ctx.response.headers.vary.includes(HeaderKeys.SEGMENT)
const setKey = varySegment ? keyWithSegment : key
const responseEncoding = configResponseEncoding || (responseType === 'arraybuffer' ? 'base64' : undefined)
const cacheableData = type === CacheType.Disk && responseType === 'arraybuffer'
Expand Down
2 changes: 1 addition & 1 deletion src/HttpClient/middlewares/request/index.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { AxiosRequestConfig } from 'axios'
import buildFullPath from '../../../utils/buildFullPath'
import { Limit } from 'p-limit'
import { stringify } from 'qs'
import { toLower } from 'ramda'
import buildFullPath from '../../../utils/buildFullPath'


import { CustomHttpTags, OpentracingTags } from '../../../tracing/Tags'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export class TestServer {
return reject(err)
}

resolve()
resolve(undefined)
})
})
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { MockSpanContext } from '@tiagonapoli/opentracing-alternate-mock'
import { AxiosError, AxiosInstance } from 'axios'
import { REFERENCE_CHILD_OF, REFERENCE_FOLLOWS_FROM } from 'opentracing'
import { ROUTER_CACHE_HEADER } from '../../../../../constants'
import { HeaderKeys } from '../../../../../constants'
import { SpanReferenceTypes } from '../../../../../tracing'
import { ErrorReportLogFields } from '../../../../../tracing/LogFields'
import { CustomHttpTags, OpentracingTags } from '../../../../../tracing/Tags'
Expand Down Expand Up @@ -31,6 +31,20 @@ export interface TestSuiteConfig {
export const registerSharedTestSuite = (testSuiteConfig: TestSuiteConfig) => {
const { axiosInstance: http } = testSuiteConfig

// Helper function to get error message that works across Node.js versions
const getErrorMessage = (error: AxiosError, expectedPrefix: string): string => {
// In Node.js 20+, message might be empty, check errors array
const errorWithErrors = error as any
if (!error.message && errorWithErrors.errors && errorWithErrors.errors.length > 0) {
// Find the error that matches our expected prefix
const matchingError = errorWithErrors.errors.find((err: any) =>
err.message && err.message.startsWith(expectedPrefix)
)
return matchingError ? matchingError.message : error.message
Comment on lines +35 to +43
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

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

[nitpick] The error handling compatibility function is well-implemented, but consider adding proper TypeScript types instead of using 'any'. Define an interface for the expected error structure to improve type safety.

Suggested change
const getErrorMessage = (error: AxiosError, expectedPrefix: string): string => {
// In Node.js 20+, message might be empty, check errors array
const errorWithErrors = error as any
if (!error.message && errorWithErrors.errors && errorWithErrors.errors.length > 0) {
// Find the error that matches our expected prefix
const matchingError = errorWithErrors.errors.find((err: any) =>
err.message && err.message.startsWith(expectedPrefix)
)
return matchingError ? matchingError.message : error.message
interface ErrorWithErrorsArray extends AxiosError {
errors?: { message?: string }[]
}
const getErrorMessage = (error: AxiosError, expectedPrefix: string): string => {
// In Node.js 20+, message might be empty, check errors array
const errorWithErrors = error as ErrorWithErrorsArray
if (!error.message && errorWithErrors.errors && errorWithErrors.errors.length > 0) {
// Find the error that matches our expected prefix
const matchingError = errorWithErrors.errors.find(
(err) => err.message && err.message.startsWith(expectedPrefix)
)
return matchingError ? matchingError.message! : error.message

Copilot uses AI. Check for mistakes.
}
return error.message
}

it('Creates the expected amount of spans', async () => {
const { allRequestSpans, tracerReport } = await TracedTestRequest.doRequest(http, testSuiteConfig.requestsConfig)
expect(allRequestSpans.length).toBe(testSuiteConfig.expects.numberOfRequestSpans)
Expand Down Expand Up @@ -141,7 +155,7 @@ export const registerSharedTestSuite = (testSuiteConfig: TestSuiteConfig) => {

if (testSuiteConfig.testServer) {
it(`Properly assigns router cache tag when it's present`, async () => {
testSuiteConfig.testServer!.mockResponseHeaders({ [ROUTER_CACHE_HEADER]: 'MISS' })
testSuiteConfig.testServer!.mockResponseHeaders({ [HeaderKeys.ROUTER_CACHE]: 'MISS' })
const { allRequestSpans } = await TracedTestRequest.doRequest(http, testSuiteConfig.requestsConfig)
allRequestSpans.forEach((requestSpan) => {
expect(requestSpan.tags()[CustomHttpTags.HTTP_ROUTER_CACHE_RESULT]).toEqual('MISS')
Expand All @@ -155,22 +169,28 @@ export const registerSharedTestSuite = (testSuiteConfig: TestSuiteConfig) => {
it('Throws an axios error', async () => {
const { error } = await TracedTestRequest.doRequest(http, testSuiteConfig.requestsConfig)
expect((error as AxiosError).isAxiosError).toBe(true)
expect((error as AxiosError).message.startsWith(testSuiteConfig.expects.error!.errorMessagePrefix)).toBeTruthy()
const errorMessage = getErrorMessage(error as AxiosError, testSuiteConfig.expects.error!.errorMessagePrefix)
expect(errorMessage.startsWith(testSuiteConfig.expects.error!.errorMessagePrefix)).toBeTruthy()
})

it('Assigns error tags and error logs to all request spans', async () => {
const { allRequestSpans } = await TracedTestRequest.doRequest(http, testSuiteConfig.requestsConfig)
const { allRequestSpans, error } = await TracedTestRequest.doRequest(http, testSuiteConfig.requestsConfig)
allRequestSpans.forEach((requestSpan) => {
expect(requestSpan!.tags()[OpentracingTags.ERROR]).toEqual('true')
const len = (requestSpan as any)._logs.length
expect((requestSpan as any)._logs[len - 1].fields.event).toEqual('error')
expect((requestSpan as any)._logs[len - 1].fields[ErrorReportLogFields.ERROR_ID]).toBeDefined()
expect((requestSpan as any)._logs[len - 1].fields[ErrorReportLogFields.ERROR_KIND]).toBeDefined()
expect(
((requestSpan as any)._logs[len - 1].fields[ErrorReportLogFields.ERROR_MESSAGE] as string).startsWith(
testSuiteConfig.expects.error!.errorMessagePrefix
)
).toBeTruthy()

// Updated error message check to handle both Node.js versions
const errorMessage = (requestSpan as any)._logs[len - 1].fields[ErrorReportLogFields.ERROR_MESSAGE] as string
const expectedPrefix = testSuiteConfig.expects.error!.errorMessagePrefix

// Check if error message starts with prefix OR if it's empty but we have the expected error code
const hasExpectedPrefix = errorMessage && errorMessage.startsWith(expectedPrefix)
const hasExpectedCode = !errorMessage && (error as AxiosError).code === 'ECONNREFUSED'
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

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

[nitpick] The hardcoded 'ECONNREFUSED' error code should be extracted to a constant or made configurable to improve maintainability and make the test expectations more explicit.

Copilot uses AI. Check for mistakes.

expect(hasExpectedPrefix || hasExpectedCode).toBeTruthy()
})
})
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { AxiosInstance, AxiosRequestConfig, AxiosResponse } from 'axios'
import buildFullPath from '../../../../../../utils/buildFullPath'
import { Span } from 'opentracing'
import { ROUTER_CACHE_HEADER } from '../../../../../../constants'
import { HeaderKeys } from '../../../../../../constants'
import { CustomHttpTags, OpentracingTags } from '../../../../../../tracing/Tags'
import { cloneAndSanitizeHeaders } from '../../../../../../tracing/utils'
import buildFullPath from '../../../../../../utils/buildFullPath'

export const injectRequestInfoOnSpan = (span: Span | undefined, http: AxiosInstance, config: AxiosRequestConfig) => {
span?.addTags({
Expand All @@ -24,7 +24,8 @@ export const injectResponseInfoOnSpan = (span: Span | undefined, response: Axios

span?.log({ 'response-headers': cloneAndSanitizeHeaders(response.headers) })
span?.setTag(OpentracingTags.HTTP_STATUS_CODE, response.status)
if (response.headers[ROUTER_CACHE_HEADER]) {
span?.setTag(CustomHttpTags.HTTP_ROUTER_CACHE_RESULT, response.headers[ROUTER_CACHE_HEADER])

if (response.headers[HeaderKeys.ROUTER_CACHE]) {
span?.setTag(CustomHttpTags.HTTP_ROUTER_CACHE_RESULT, response.headers[HeaderKeys.ROUTER_CACHE])
}
}
Loading