fix(nodejs): dedupe @smithy/core to fix AWS error instanceof checks - #64488
Conversation
…checks Multiple @smithy/core copies (3.21.1/3.22.1/3.24.6) split the schema TypeRegistry across module realms, so @aws-sdk/client-* registered error classes into a different realm than the protocol deserializer read from. Lookups missed and the SDK fell back to a bare new Error(errorCode), defeating instanceof on every AWS error — surfacing as ConditionalCheckFailedException leaking out of DynamoDBKeyStore.generateKey. Pin @smithy/core via pnpm.overrides so the whole tree shares one TypeRegistry, which restores proper error class identity. With instanceof reliable again, revert the keystore and recording-service catches to their original simple instanceof checks (the .name workarounds from #64376 were ineffective anyway — the code lands in .message, not .name).
👀 Auto-assigned reviewersThese soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:
Soft owners come from |
|
Reviews (1): Last reviewed commit: "fix(nodejs): dedupe @smithy/core to one ..." | Re-trigger Greptile |
|
Size Change: 0 B Total Size: 64.3 MB ℹ️ View Unchanged
|
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
ghost
left a comment
There was a problem hiding this comment.
LGTM
The fix correctly identifies and addresses the root cause: multiple @smithy/core copies creating separate TypeRegistry singletons. The override is the right lever — it's minimal, the lockfile confirms deduplication, and the reverted instanceof checks are now safe. No correctness, security, or performance issues found.
What this PR does
Pins @smithy/core to a single version (3.24.6) via pnpm.overrides to eliminate multiple TypeRegistry realms that broke instanceof checks for all AWS SDK error classes. Reverts the .name-based workarounds from #64376 back to simple instanceof checks now that the root cause is fixed.
Tag @mendral-app with feedback or questions. View session
Problem
ConditionalCheckFailedExceptionwas leaking out ofDynamoDBKeyStore.generateKeyto prod — a benign concurrent-write race the catch is meant to swallow.Root cause: the
nodejstree had multiple@smithy/corecopies (3.21.1 / 3.22.1 / 3.24.6). The schemaTypeRegistryis a per-copy module singleton, so@aws-sdk/client-*registered its error classes into one realm while the protocol deserializer looked them up in another. The lookup missed, the SDK couldn't even resolve the synthetic base exception, and it fell back tonew Error(errorCode)— a bareErrorwith the code in.messageand.name === 'Error'. That breaksinstanceoffor every AWS error in the service, not just this one.Changes
@smithy/coreto a single version viapnpm.overrides, so the whole tree shares oneTypeRegistryand error classes keep their identity. The lockfile now resolves every@aws-sdk/client-*and@aws-sdk/coreto@smithy/core@3.24.6.instanceofreliable again, revert the keystore andrecording-servicecatches to their original simpleinstanceofchecks. The.nameworkarounds added in chore(recording-api): fix aws error handling #64376 never worked anyway (the code lands in.message).This is the durable fix for the class of bug that #64482 patched at individual call sites — once this lands, #64482 can be closed.
How did you test this code?
Agent (Claude Code), automated only. Verified the lockfile collapses to one
@smithy/core@3.24.6and that every AWS client +@aws-sdk/corenow resolves to it.dynamodb-keystore+recording-servicesuites green (74/74), tsc clean for the AWS code.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Single-version override is the minimal lever —
TypeRegistrylives in@smithy/core, so collapsing just that package unifies the realm regardless of@aws-sdk/coreduplication. Reverting to plaininstanceofkeeps the error handling idiomatic now that the SDK behaves.