Maintenance: make the stores' by-value guarantee explicit and opt-in

[svozza]

Summary

Part of the original intent behind the *Store classes was that state shared across invocations is reachable only through a store, and that stores hand out values rather than references — so a caller can't retain a handle on invocation-scoped state and mutate it from somewhere else. Under Lambda Managed Instances, where invocations run concurrently in the same execution environment, that removes a class of concurrency bug by construction rather than by discipline.

Today that guarantee is a convention enforced by hand at each accessor, and it holds unevenly. It's strongest in metrics' DimensionsStore (copy on read, per-element copy for nested structures, copy on write) and in logger's attribute accessors, absent in batch, and inconsistent within a single file. This issue proposes making the policy explicit — declared per value, layered on top of the primitive from #5554 by composition — and deciding the cases where it currently doesn't hold.

Why is this needed?

The policy is invisible at the point where it matters, so it drifts. MetricsStore is the clearest example of the cost — setMetric() is meticulous, returning { ...newMetric } and, for the append case, { ...existingMetric, value: [...existingMetric.value] } so both levels are copied (MetricsStore.ts:86,100), while getMetric() two dozen lines above returns the live StoredMetric straight out of storage (MetricsStore.ts:45-47) and getAllMetrics() returns a fresh array whose elements are all live references (MetricsStore.ts:107-109). Nothing in the code signals which of those was a decision and which was an oversight.

The gap with real exposure is batch: BatchProcessingStore neither copies on read nor on write, and those values are surfaced through public getters on BasePartialProcessor (.errors, .failureMessages, .records, .successMessages, BasePartialProcessor.ts:29-73), so a custom processor can do processor.errors.push(...) and mutate invocation-scoped state directly.

Solution

Keep the primitive from #5554 reference-based, and add by-value behaviour as a wrapper that requires a copy function in order to exist:

/**
 * Adds by-value semantics to an invocation-scoped cell, so that callers never
 * receive a reference to the stored value.
 */
class ByValue<T> {
  constructor(cell: InvocationScoped<T>, copy: (value: T) => T);

  /** The live value, for in-place mutation within the store. */
  mutable(): T;

  /** A copy of the value, safe to hand out. */
  snapshot(): T;

  /** Stores a copy, so the caller retains no reference into the cell. */
  set(value: T): void;
}

In a store, the copy policy is then declared once next to the value, and the call sites read as the intent:

class LogAttributesStore {
  readonly #temporaryAttributes = new ByValue(
    new InvocationScoped<LogAttributes>('powertools.logger.temporaryAttributes', {
      fresh: () => ({}),
    }),
    (attributes) => ({ ...attributes })
  );

  public appendTemporaryKeys(attributes: LogAttributes): void {
    deepMerge(this.#temporaryAttributes.mutable(), attributes); // in-place, stays internal
    // ...keys bookkeeping
  }

  public getTemporaryAttributes(): LogAttributes {
    return this.#temporaryAttributes.snapshot(); // handed out, so copied
  }
}

Batch keeps the bare cell, which is what makes the opt-in work — there is no snapshot() on it to call by mistake, and nothing copies on the record-processing path:

readonly #records = new InvocationScoped<BaseRecord[]>('powertools.batch.records', {
  fresh: () => [],
});

Copy depth stays as it is today ({ ...attrs }, not structuredClone), so nested value references behave identically. The type-level part is the point: "I called the safe accessor but never declared a copy function" becomes a compile error rather than a silent leak.

Why a wrapper rather than a subclass. class ByValue<T> extends InvocationScoped<T> would inherit the live accessor as a public member, and TypeScript won't let a subclass reduce an inherited member's visibility — so the unsafe accessor stays reachable on the very type whose purpose is to not expose it. Removing an operation from a surface is a restriction, not an extension, which is the usual sign the relationship isn't is-a. It would also force the primitive's # privates open to protected (rendered in the API reference, since typedoc.json sets visibilityFilters.protected: true) and make its method signatures a variance-constrained supertype across package boundaries, so a change in commons could break subclasses in logger and metrics. Composition keeps the shared surface minimal and matches the existing layering, where BasePartialProcessor and Logger hold their stores rather than extend them. The wrapper doesn't have to be a class at all — a factory returning a frozen object closing over the cell and the copy function gives the same type-level guarantee.

Where it lives. Only logger and metrics need it, so it can start as a small local helper in each rather than widening commons' semi-public surface for sugar that has no environment awareness. If the two copies start to drift, promoting it to commons later is mechanical.

Per-value decisions. Each of these is a behaviour change on a public surface and should be settled individually:

• LogAttributesStore.getLambdaContext() returns the stored object live (LogAttributesStore.ts:128-134). Callers happen to be safe today — createChild rebuilds a fresh literal via addContext, and #getPowertoolsLogData spreads it into a new object while keeping the nested reference — but nothing enforces that.
• MetricsStore.getMetric() and getAllMetrics(), per above.
• BatchProcessingStore: copying records on every read would clone the batch array per record in the processing loop, and SqsFifoProcessorStore.addFailedGroupId/hasFailedGroupId are built on mutating the live Set from getFailedGroupIds(). Batch is why the guarantee can't be baked into the primitive; the likely outcome is that batch declares nothing and keeps today's behaviour, but the public getters deserve an explicit decision rather than an implicit one.
• The log buffer's SizedSet from LogInvocationStore.get(traceId) should stay live — the flush path only iterates it and then deletes the trace id, so copying a buffer in order to print it is pure waste.

Depends on #5554, which moves the mode branch only and leaves every store's current copy behaviour verbatim.

Which area does this relate to?

Commons, Logger, Metrics, Batch Processing

Acknowledgment

• This request meets Powertools for AWS Lambda (TypeScript) Tenets
• Should this be considered in other Powertools for AWS Lambda languages? i.e. Python, Java, and .NET

Future readers

Please react with 👍 and your use case to help us understand customer demand.