Skip to content

fix: dispatch handlers via live lookup instead of .bind() snapshot - #109

Open
bro-ankit wants to merge 1 commit into
ssut:masterfrom
bro-ankit:fix/live-lookup-handler-dispatch
Open

fix: dispatch handlers via live lookup instead of .bind() snapshot#109
bro-ankit wants to merge 1 commit into
ssut:masterfrom
bro-ankit:fix/live-lookup-handler-dispatch

Conversation

@bro-ankit

@bro-ankit bro-ankit commented Jul 3, 2026

Copy link
Copy Markdown

Summary

Fixes #108SqsService binds @SqsMessageHandler/@SqsConsumerEventHandler methods via .bind() at onModuleInit, capturing a frozen snapshot of the method at that exact moment. Any provider that wraps a decorated method after that point — a standard Nest extension pattern (see @golevelup/nestjs-discovery's own docs, which cite @nestjs/graphql's @Mutation/@Resolver discovery as the reference example) — has that wrapping silently ignored for SQS-bound handlers. The message still processes (nothing crashes), but any cross-cutting logic layered on top never fires. No error, no warning.

This isn't a contrived timing edge case: onModuleInit runs strictly before onApplicationBootstrap for the entire application, across every module, regardless of import order. Any executor using OnApplicationBootstrap (a common choice, since it's guaranteed to run after all onModuleInit hooks) hits this on every SQS handler, unconditionally.

Fix

Replaced the .bind() capture (handleMessage, handleMessageBatch, event listener) with a closure that looks up instance[methodName] at call time:

// before
handleMessage: metadata.discoveredMethod.handler.bind(metadata.discoveredMethod.parentClass.instance)

// after
handleMessage: (...args) => instance[methodName](...args)

@bro-ankit bro-ankit left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@ssut could you take a look when you get a chance?

@bro-ankit bro-ankit left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@ssut Hi! Just a friendly follow-up on this PR. I can work around it with a local patch, but I'd prefer to get this upstream instead. I'd appreciate a review when you have a chance. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SqsService binds handlers via .bind() at onModuleInit, silently breaking any post-boot method wrapping (metrics/tracing/@Transactional-style decorators)

1 participant