Skip to content

Refactor duplicate event registration error handling #987

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DZakh merged 13 commits into from
Feb 25, 2026
Merged

Refactor duplicate event registration error handling #987

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6427379
Detect duplicate event handler registrations with clear JS errors
claude Feb 24, 2026
784da71
Add lock file updates from dependency installation
claude Feb 24, 2026
65d3697
Remove .npmrc that was a local build workaround
claude Feb 24, 2026
9cbeadb
Merge origin/main into claude/detect-duplicate-handlers-Km3yA
claude Feb 24, 2026
af1c34d
Allow duplicate handler registration with same options by composing them
claude Feb 24, 2026
4dc0a64
Update test_codegen pnpm-lock.yaml
claude Feb 24, 2026
f059333
Merge branch 'main' into claude/detect-duplicate-handlers-Km3yA
DZakh Feb 24, 2026
850ccf1
Use === (referential equality) for eventOptions comparison
claude Feb 24, 2026
f1c4073
Use === only for wildcard (bool), keep == for eventFilters (structural)
claude Feb 24, 2026
7baf435
Use === for eventFilters too to avoid caml_equal throw on functions
claude Feb 24, 2026
7e0f733
Use == for eventFilters (Js.Json.t, no functions) and === for wildcar…
claude Feb 25, 2026
74f4d4f
Use Internal.eventFilters instead of Js.Json.t for eventOptions type
claude Feb 25, 2026
bd2dd9a
Validate eventOptions before writing handler to avoid partial writes
claude Feb 25, 2026
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
127 changes: 92 additions & 35 deletions packages/envio/src/HandlerRegister.res
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
type eventRegistration = {
handler: option<Internal.handler>,
contractRegister: option<Internal.contractRegister>,
eventOptions: option<Internal.eventOptions<Js.Json.t>>,
eventOptions: option<Internal.eventOptions<Internal.eventFilters>>,
}

let empty = {
Expand Down Expand Up @@ -175,6 +175,7 @@ let getContractRegister = (~contractName, ~eventName) =>
let getEventFilters = (~contractName, ~eventName) =>
get(~contractName, ~eventName).eventOptions
->Belt.Option.flatMap(value => value.eventFilters)
->(Utils.magic: option<Internal.eventFilters> => option<Js.Json.t>)

let isWildcard = (~contractName, ~eventName) =>
get(~contractName, ~eventName).eventOptions
Expand All @@ -187,26 +188,49 @@ let hasRegistration = (~contractName, ~eventName) => {
}

type eventNamespace = {contractName: string, eventName: string}
exception DuplicateEventRegistration(eventNamespace)

let raiseDuplicateRegistration = (~contractName, ~eventName, ~msg, ~logger) => {
let fullMsg = msg ++ " for " ++ contractName ++ "." ++ eventName
Logging.createChildFrom(~logger, ~params={contractName, eventName})->Logging.childError(fullMsg)
Js.Exn.raiseError(fullMsg)
}
Comment on lines +192 to +196

@coderabbitai coderabbitai Bot Feb 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use an object literal for logger params payload.

~params={contractName, eventName} is a record literal; prefer a JS object for payloads passed between functions.

🛠️ Suggested change 
-  Logging.createChildFrom(~logger, ~params={contractName, eventName})->Logging.childError(fullMsg)
+  Logging.createChildFrom(
+    ~logger,
+    ~params={"contractName": contractName, "eventName": eventName},
+  )->Logging.childError(fullMsg)
As per coding guidelines, "Use records when working with structured data, and objects to conveniently pass payload data between functions".
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/envio/src/HandlerRegister.res` around lines 191 - 195, The params
passed into Logging.createChildFrom are currently a record literal
({contractName, eventName}); change this to a JS object payload instead so
downstream JS consumers get an object. Update the call in
raiseDuplicateRegistration (the ~params argument passed to
Logging.createChildFrom / Logging.childError) to pass an object-style payload
mapping contractName and eventName to their values (i.e., a JS object with keys
"contractName" and "eventName") rather than the record literal.


let eventFiltersMatch = (a: option<Internal.eventFilters>, b: option<Internal.eventFilters>) => {
switch (a, b) {
| (None, None) => true
| (Some(Static(a)), Some(Static(b))) => a == b
| (Some(Dynamic(a)), Some(Dynamic(b))) => a === b
| _ => false
}
}

let eventOptionsMatch = (
existing: option<Internal.eventOptions<Internal.eventFilters>>,
incoming: option<Internal.eventOptions<Internal.eventFilters>>,
) => {
switch (existing, incoming) {
| (None, None) => true
| (Some(a), Some(b)) =>
a.wildcard === b.wildcard && eventFiltersMatch(a.eventFilters, b.eventFilters)
| _ => false
}
Comment on lines +207 to +216

@coderabbitai coderabbitai Bot Feb 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find eventOptions type definition and eventFilters references
rg -n "type eventOptions" -g '*.res' -A 5

Repository: enviodev/hyperindex

Length of output: 378

🏁 Script executed:

#!/bin/bash
# Search for eventFilters usage and definition
rg -n "eventFilters" -g '*.res' -B 2 -A 2

Repository: enviodev/hyperindex

Length of output: 11204

🏁 Script executed:

#!/bin/bash
# Check the Internal module to find the actual type definition
fd -e res -e resi | head -20 | xargs grep -l "eventOptions\|eventFilters" 2>/dev/null

Repository: enviodev/hyperindex

Length of output: 98

Use deep equality or stable comparison for eventFilters matching.

The eventFilters field is cast from a typed variant (Static with arrays or Dynamic with functions) to opaque Js.Json.t before comparison. Comparing with == on Js.Json.t will reject structurally identical filters if they are different object references. Since the error message directs users to use "identical options," implement deep-equality comparison or document that eventFilters must be the exact same reference across registrations.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/envio/src/HandlerRegister.res` around lines 197 - 205, The current
eventOptionsMatch function compares eventFilters (of type Js.Json.t) with
reference equality, which fails for structurally equal JSON; update
eventOptionsMatch (the function comparing Internal.eventOptions) to perform a
deep/stable comparison of a.eventFilters and b.eventFilters instead of using ==
(for example by canonicalizing to a stable string representation like JSON
stringify or calling a dedicated deepEqual helper) while keeping the wildcard
equality check; ensure the deep comparison handles None/Some cases the same way
as now and applies only to the eventFilters field of Internal.eventOptions.

}

let setEventOptions = (~contractName, ~eventName, ~eventOptions, ~logger=Logging.getLogger()) => {
switch eventOptions {
| Some(value) =>
let value =
value->(Utils.magic: Internal.eventOptions<'eventFilters> => Internal.eventOptions<Js.Json.t>)
value->(Utils.magic: Internal.eventOptions<'eventFilters> => Internal.eventOptions<Internal.eventFilters>)
let t = get(~contractName, ~eventName)
switch t.eventOptions {
| None => set(~contractName, ~eventName, {...t, eventOptions: Some(value)})
| Some(existingValue) =>
if (
existingValue.wildcard !== value.wildcard ||
// TODO: Can improve the check by using deepEqual
existingValue.eventFilters !== value.eventFilters
) {
let eventNamespace = {contractName, eventName}
DuplicateEventRegistration(eventNamespace)->ErrorHandling.mkLogAndRaise(
~logger=Logging.createChildFrom(~logger, ~params=eventNamespace),
~msg="Duplicate eventOptions in handlers not allowed",
if !eventOptionsMatch(Some(existingValue), Some(value)) {
raiseDuplicateRegistration(
~contractName,
~eventName,
~msg="Cannot register handler with different options. Make sure all handlers for the same event use identical options (wildcard, eventFilters)",
~logger,
)
}
}
Expand All @@ -217,46 +241,79 @@ let setEventOptions = (~contractName, ~eventName, ~eventOptions, ~logger=Logging
let setHandler = (~contractName, ~eventName, handler, ~eventOptions, ~logger=Logging.getLogger()) => {
withRegistration(_registration => {
let t = get(~contractName, ~eventName)
let newHandler = handler->(Utils.magic: Internal.genericHandler<'args> => Internal.handler)
switch t.handler {
| None =>
setEventOptions(~contractName, ~eventName, ~eventOptions, ~logger)
let t = get(~contractName, ~eventName)
set(~contractName, ~eventName, {
...t,
handler: handler
->(Utils.magic: Internal.genericHandler<'args> => Internal.handler)
->Some,
handler: Some(newHandler),
})
| Some(_) =>
let eventNamespace = {contractName, eventName}
DuplicateEventRegistration(eventNamespace)->ErrorHandling.mkLogAndRaise(
~logger=Logging.createChildFrom(~logger, ~params=eventNamespace),
~msg="Duplicate registration of event handlers not allowed",
)
| Some(prevHandler) =>
Comment thread
coderabbitai[bot] marked this conversation as resolved.
let incomingEventOptions =
eventOptions->Belt.Option.map(v =>
v->(Utils.magic: Internal.eventOptions<'eventFilters> => Internal.eventOptions<Internal.eventFilters>)
)
if eventOptionsMatch(t.eventOptions, incomingEventOptions) {
let composedHandler: Internal.handler = async args => {
await prevHandler(args)
await newHandler(args)
}
set(~contractName, ~eventName, {
...t,
handler: Some(composedHandler),
})
} else {
raiseDuplicateRegistration(
~contractName,
~eventName,
~msg="Cannot register a second handler with different options. Make sure all handlers for the same event use identical options (wildcard, eventFilters)",
~logger,
)
}
}

setEventOptions(~contractName, ~eventName, ~eventOptions, ~logger)
})
}

let setContractRegister = (~contractName, ~eventName, contractRegister, ~eventOptions, ~logger=Logging.getLogger()) => {
withRegistration(_registration => {
let t = get(~contractName, ~eventName)
let newContractRegister = contractRegister->(
Utils.magic: Internal.genericContractRegister<
Internal.genericContractRegisterArgs<'event, 'context>,
> => Internal.contractRegister
)
switch t.contractRegister {
| None =>
setEventOptions(~contractName, ~eventName, ~eventOptions, ~logger)
let t = get(~contractName, ~eventName)
set(~contractName, ~eventName, {
...t,
contractRegister: contractRegister->(
Utils.magic: Internal.genericContractRegister<
Internal.genericContractRegisterArgs<'event, 'context>,
> => Internal.contractRegister
)->Some,
contractRegister: Some(newContractRegister),
})
| Some(_) =>
let eventNamespace = {contractName, eventName}
DuplicateEventRegistration(eventNamespace)->ErrorHandling.mkLogAndRaise(
~logger=Logging.createChildFrom(~logger, ~params=eventNamespace),
~msg="Duplicate contractRegister handlers not allowed",
)
| Some(prevContractRegister) =>
let incomingEventOptions =
eventOptions->Belt.Option.map(v =>
v->(Utils.magic: Internal.eventOptions<'eventFilters> => Internal.eventOptions<Internal.eventFilters>)
)
if eventOptionsMatch(t.eventOptions, incomingEventOptions) {
let composedContractRegister: Internal.contractRegister = async args => {
await prevContractRegister(args)
await newContractRegister(args)
}
set(~contractName, ~eventName, {
...t,
contractRegister: Some(composedContractRegister),
})
} else {
raiseDuplicateRegistration(
~contractName,
~eventName,
~msg="Cannot register a second contractRegister with different options. Make sure all handlers for the same event use identical options (wildcard, eventFilters)",
~logger,
)
}
}
setEventOptions(~contractName, ~eventName, ~eventOptions, ~logger)
})
}
Loading