feat: make saveLastProcessedEvent part of transaction #60
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import { | ||
| IEventRegistryRepository, | ||
| ProcessedEventChangeset, | ||
| } from "@grants-stack-indexer/repository"; | ||
|
|
||
| import { ChangesetHandler } from "../types/index.js"; | ||
|
|
||
| /** | ||
| * Collection of handlers for application-related operations. | ||
| * Each handler corresponds to a specific Application changeset type. | ||
| */ | ||
| export type ProcessedEventHandlers = { | ||
| [K in ProcessedEventChangeset["type"]]: ChangesetHandler<K>; | ||
| }; | ||
|
|
||
| /** | ||
| * Creates handlers for managing application-related operations. | ||
| * | ||
| * @param repository - The application repository instance used for database operations | ||
| * @returns An object containing all application-related handlers | ||
| */ | ||
| export const createProcessedEventHandlers = ( | ||
| repository: IEventRegistryRepository, | ||
| ): ProcessedEventHandlers => ({ | ||
| InsertProcessedEvent: (async (changeset, txConnection): Promise<void> => { | ||
| await repository.saveLastProcessedEvent( | ||
| changeset.args.chainId, | ||
| changeset.args.processedEvent, | ||
| txConnection, | ||
| ); | ||
| }) satisfies ChangesetHandler<"InsertProcessedEvent">, | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import { describe, expect, it, vi } from "vitest"; | ||
|
|
||
| import { IEventRegistryRepository, TransactionConnection } from "@grants-stack-indexer/repository"; | ||
| import { ChainId } from "@grants-stack-indexer/shared"; | ||
|
|
||
| import { createProcessedEventHandlers } from "../../../src/data-loader/handlers/processedEvent.handlers.js"; | ||
|
|
||
| describe("ProcessedEvent Handlers", () => { | ||
| const chainId = 1 as ChainId; | ||
| const mockEvent = { | ||
| blockNumber: 1, | ||
| blockTimestamp: 1234567890, | ||
| logIndex: 0, | ||
| rawEvent: {}, | ||
| }; | ||
|
|
||
| describe("InsertProcessedEvent", () => { | ||
| it("saves event to repository within transaction", async () => { | ||
| const saveLastProcessedEvent = vi.fn(); | ||
| const mockRepository = { | ||
| saveLastProcessedEvent, | ||
| getLastProcessedEvent: vi.fn(), | ||
| } as unknown as IEventRegistryRepository<TransactionConnection>; | ||
|
|
||
| const handlers = createProcessedEventHandlers(mockRepository); | ||
|
|
||
| const mockTx = {} as TransactionConnection; | ||
|
|
||
| await handlers.InsertProcessedEvent( | ||
| { | ||
| type: "InsertProcessedEvent", | ||
| args: { | ||
| chainId, | ||
| processedEvent: mockEvent, | ||
| }, | ||
| }, | ||
| mockTx, | ||
| ); | ||
|
|
||
| expect(saveLastProcessedEvent).toHaveBeenCalledWith(chainId, mockEvent, mockTx); | ||
| }); | ||
|
|
||
| it("propagates repository errors", async () => { | ||
| const error = new Error("Database error"); | ||
| const saveLastProcessedEvent = vi.fn().mockRejectedValue(error); | ||
| const mockRepository = { | ||
| saveLastProcessedEvent, | ||
| getLastProcessedEvent: vi.fn(), | ||
| } as unknown as IEventRegistryRepository<TransactionConnection>; | ||
|
|
||
| const handlers = createProcessedEventHandlers(mockRepository); | ||
|
|
||
| const mockTx = {} as TransactionConnection; | ||
|
|
||
| await expect( | ||
| handlers.InsertProcessedEvent( | ||
| { | ||
| type: "InsertProcessedEvent", | ||
| args: { | ||
| chainId, | ||
| processedEvent: mockEvent, | ||
| }, | ||
| }, | ||
| mockTx, | ||
| ), | ||
| ).rejects.toThrow(error); | ||
| }); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle potential null reference in
eventThere is a risk of a null or undefined reference when using
event!in the async function passed tothis.retryHandler.execute. Ensure thateventis properly checked or handled to avoid runtime errors.Apply this diff to add null checking:
await this.retryHandler.execute( async () => { - const changesets = await this.handleEvent(event!); + if (!event) { + throw new Error("Event is undefined or null"); + } + const changesets = await this.handleEvent(event); if (changesets) { await this.dataLoader.applyChanges([ ...changesets, { type: "InsertProcessedEvent", args: { chainId: this.chainId, processedEvent: { - ...event!, + ...event, rawEvent: event, }, }, }, ]); } }, { abortSignal: signal }, );📝 Committable suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
look at the prior lines, if event is undefined or null, we
continuethe loop prior to the the retryHandler execute method. this doesn't need to be fixedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@0xnigir1 You're absolutely right. I apologize for the unnecessary suggestion. The code already has a proper null check before the
retryHandler.executecall, which ensures thateventis never null or undefined at that point.✏️ Learnings added