Skip to content

Conversation

@bradcypert
Copy link
Contributor

Potential Fix for #25970.

Fixes an issue where Veb would silently fail to bind a handler when the return type of a handler function is not veb.Result (and is, for example, !veb.Result). I've also added tests to cover this. I'm happy to take a different approach if needed, too. This is my first contribution to V and am still learning the ropes.

Changes:

  • Added a function to check for route attributes which is used in other items below
  • When generating routes, adds a branch to handle the case where a method does not have a veb.Result return type, but does have route attributes, in which case, error to warn the user of unexpected behavior.

Copy link
Member

@spytheman spytheman left a comment

Choose a reason for hiding this comment

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

Welcome to the V repo, and thanks for your excellent PR.

I think it is a very good addition, and it can help prevent a lot of confusion (as seen on Discord and here).

@spytheman
Copy link
Member

I played a bit with it, and discovered, that we are missing a source location information in the metadata for the method, but it would have been useful for situations like this 🤔 .

For example, I got:

V panic: method `get_task` has route attributes but invalid return type. Handler methods must return `veb.Result`, not `!veb.Result` or other types

If we had a location information, then the error message could become:

V panic: method `vvv.v:13:18:get_task` has route attributes but invalid return type. Handler methods must return `veb.Result`, not `!veb.Result` or other types

I'll see what can be done about it asap.

@spytheman spytheman merged commit a8bded8 into vlang:master Dec 15, 2025
72 checks passed
@spytheman
Copy link
Member

With latest V 22fafd6, the error is now:

method `get_task` at `vlib/veb/tests/invalid_return_type_test.v:25:18` has route attributes but invalid return type. Handler methods must return `veb.Result`, not `!veb.Result` or other types

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.

2 participants