Skip to content

Commit 9c13632

Browse files
committed
revert(observability): drop the global-state normalization machinery
Reverts the previous two commits. They guarded a case that only arises when a dev server's globalThis outlives a module evaluation across this change, where the cost is a NaN in a diagnostic log locally — and the guard itself was worth more risk than the thing it prevented, having briefly replaced the shared state object and so allowed two module instances to each open their own connection. Back to the original initialize-once block with the new fields added. The global-state handling is now identical to staging apart from those additions.
1 parent 13e6db8 commit 9c13632

3 files changed

Lines changed: 17 additions & 104 deletions

File tree

apps/sim/lib/billing/calculations/usage-reservation.test.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -339,13 +339,6 @@ describe('usage-reservation', () => {
339339
})
340340

341341
describe('refreshExecutionSlotExpiry', () => {
342-
it('rethrows the original error object rather than the diagnostic wrapper', async () => {
343-
const original = Object.assign(new Error('Command timed out'), { code: 'ETIMEDOUT' })
344-
getMock.mockRejectedValueOnce(original)
345-
346-
await expect(refreshExecutionSlotExpiry('exec-1', Date.now() + 60_000)).rejects.toBe(original)
347-
})
348-
349342
it('refreshes only the locally owned slot and matching pointer', async () => {
350343
evalMock.mockResolvedValueOnce(1).mockResolvedValueOnce(1)
351344
await reserveExecutionSlot(memberParams)

apps/sim/lib/core/config/redis.test.ts

Lines changed: 0 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import {
3030
describeRedisConnection,
3131
extendLock,
3232
getRedisClient,
33-
normalizeRedisState,
3433
onRedisReconnect,
3534
resetForTesting,
3635
} from '@/lib/core/config/redis'
@@ -226,66 +225,6 @@ describe('redis config', () => {
226225
expect(JSON.stringify(d)).not.toContain('10.0.0.5')
227226
})
228227

229-
it('never throws, so it cannot mask the error it is describing', () => {
230-
// Called from catch blocks: a throw here would replace the real failure.
231-
mockEnv.REDIS_URL = undefined
232-
expect(() => describeRedisConnection()).not.toThrow()
233-
234-
mockEnv.REDIS_URL = 'not a url'
235-
expect(() => describeRedisConnection()).not.toThrow()
236-
expect(describeRedisConnection().hostKind).toBe('unknown')
237-
238-
// rediss:// to a bare IP with no REDIS_TLS_SERVERNAME makes the real
239-
// getConfiguredRedisUrl path throw; the snapshot must still come back.
240-
mockEnv.REDIS_URL = 'rediss://10.0.0.5:6379'
241-
mockEnv.REDIS_TLS_SERVERNAME = undefined
242-
expect(() => describeRedisConnection()).not.toThrow()
243-
})
244-
245-
it('upgrades a state object that predates these fields instead of trusting its shape', () => {
246-
// Exactly the shape the global held before this change.
247-
const stale = {
248-
client: null,
249-
pingFailures: 0,
250-
pingInterval: null,
251-
pingInFlight: false,
252-
reconnectListeners: [],
253-
}
254-
255-
const normalized = normalizeRedisState(stale)
256-
257-
// Identity must survive: every module instance has to keep pointing at the
258-
// same object, or two of them each open their own client.
259-
expect(normalized).toBe(stale)
260-
261-
// Undefined slips past the `=== null` guards and yields NaN downstream.
262-
expect(normalized.clientCreatedAt).toBeNull()
263-
expect(normalized.lastReadyAt).toBeNull()
264-
expect(normalized.lastPingOkAt).toBeNull()
265-
expect(normalized.connects).toBe(0)
266-
expect(normalized.reconnects).toBe(0)
267-
expect(normalized.errors).toBe(0)
268-
expect(normalized.lastErrorMessage).toBeNull()
269-
for (const [key, value] of Object.entries(normalized)) {
270-
expect(value, `${key} is undefined`).not.toBeUndefined()
271-
}
272-
})
273-
274-
it('carries a running health check across normalization so it is not started twice', () => {
275-
const interval = setInterval(() => {}, 1_000)
276-
try {
277-
expect(normalizeRedisState({ pingInterval: interval }).pingInterval).toBe(interval)
278-
} finally {
279-
clearInterval(interval)
280-
}
281-
})
282-
283-
it('preserves accumulated counters when the shape is already current', () => {
284-
const normalized = normalizeRedisState({ connects: 4, reconnects: 2, errors: 7 })
285-
286-
expect(normalized).toMatchObject({ connects: 4, reconnects: 2, errors: 7 })
287-
})
288-
289228
it('does not date a connection that has been discarded', async () => {
290229
mockRedisInstance.status = 'ready'
291230
getRedisClient()

apps/sim/lib/core/config/redis.ts

Lines changed: 17 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -69,43 +69,24 @@ interface RedisState {
6969
lastErrorMessage: string | null
7070
}
7171

72-
const g = globalThis as typeof globalThis & { _redisState?: Partial<RedisState> }
73-
74-
/**
75-
Backfilled in place, because the global outlives a module evaluation in two
76-
* directions at once.
77-
*
78-
* A state object created before any of these fields existed keeps them
79-
* `undefined`, which slips past the `=== null` guards below and turns every age
80-
* into `now - undefined` and every counter into `undefined++` — NaN in both
81-
* cases, serialized as `null`, in the one payload whose purpose is being
82-
* trustworthy. So the shape has to be upgraded rather than trusted.
83-
*
84-
* It must be upgraded *without replacing the object*. Sharing one object across
85-
* evaluations is the entire reason this lives on `globalThis`: assigning a fresh
86-
* one leaves an earlier module instance holding the previous object, and two
87-
* instances that disagree about `client` each open their own connection and
88-
* start their own health check. Mutating in place keeps every holder pointing at
89-
* the same state while still filling in what an older shape lacks.
90-
*/
91-
export function normalizeRedisState(existing: Partial<RedisState>): RedisState {
92-
existing.client ??= null
93-
existing.pingFailures ??= 0
94-
existing.pingInterval ??= null
95-
existing.pingInFlight ??= false
96-
existing.reconnectListeners ??= []
97-
existing.clientCreatedAt ??= null
98-
existing.lastReadyAt ??= null
99-
existing.lastPingOkAt ??= null
100-
existing.connects ??= 0
101-
existing.reconnects ??= 0
102-
existing.errors ??= 0
103-
existing.lastErrorMessage ??= null
104-
return existing as RedisState
72+
const g = globalThis as typeof globalThis & { _redisState?: RedisState }
73+
if (!g._redisState) {
74+
g._redisState = {
75+
client: null,
76+
pingFailures: 0,
77+
pingInterval: null,
78+
pingInFlight: false,
79+
reconnectListeners: [],
80+
clientCreatedAt: null,
81+
lastReadyAt: null,
82+
lastPingOkAt: null,
83+
connects: 0,
84+
reconnects: 0,
85+
errors: 0,
86+
lastErrorMessage: null,
87+
}
10588
}
106-
107-
g._redisState ??= {}
108-
const state = normalizeRedisState(g._redisState)
89+
const state = g._redisState
10990

11091
/**
11192
* A command that never gets a reply fails identically whichever of three states

0 commit comments

Comments
 (0)