Skip to content
New issue

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

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

Already on GitHub? # to your account

Refactor: Make ValidationErrorReporter interface better. #3438

Merged
merged 2 commits into from
Jan 30, 2025

Conversation

kentonv
Copy link
Member

@kentonv kentonv commented Jan 30, 2025

This interface evolved from early beginnings where we didn't really know where we were going, and as a result was quite warty in the way that it reported entrypoints:

  • It would report each method of stateless entrypoints with a separate call, rather than reporting the whole entrypoint at once. All of the implementations had to group these back into entrypoints using HashMaps.
  • It reported a Durable Object class as if it were an entrypoint with a single method called "class". This sort of made sense back when there was a fixed list of handler names, but the introduction of RPC made this very weird.

Evidence of why this was a bad interface can be seen in the first commit of this PR, which introduces tests for three different bugs that I discovered just while refactoring this interface.

In any case, this changes the interface to just make sense.

Historically, this did not cause a config-time error, only a runtime error. Probably nobody depends on this bug, but I'd prefer not to have to roll back this change if I'm wrong, so I'll maintain the behavior for now.
@kentonv kentonv requested review from a team as code owners January 30, 2025 00:53
@jasnell
Copy link
Member

jasnell commented Jan 30, 2025

Looks like you're missing a virtual keyword in there somewhere. Signed off because the overall change looks good once the compile bugs are addressed.

This interface evolved from early beginnings where we didn't really know where we were going, and as a result was quite warty in the way that it reported entrypoints:
* It would report each _method_ of stateless entrypoints with a separate call, rather than reporting the whole entrypoint at once. All of the implementations had to group these back into entrypoints using HashMaps.
* It reported a Durable Object class as if it were an entrypoint with a single method called "class". This sort of made sense back when there was a fixed list of handler names, but the introduction of RPC made this very weird.

Evidence of why this was a bad interface can be seen in the previous commit, which introduces tests for three different bugs that I discovered just while refactoring this interface.

In any case, this changes the interface to just make sense.
@kentonv kentonv force-pushed the kenton/refactor-validation branch from 183b448 to 842d84c Compare January 30, 2025 14:13
@kentonv kentonv merged commit 789a13a into main Jan 30, 2025
17 checks passed
@kentonv kentonv deleted the kenton/refactor-validation branch January 30, 2025 23:26
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants