Skip to content

Add support for procs in default value #959

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.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

seriousdev-gh
Copy link

Grape supports lambda and procs as default values, it evaluates them for each request. But grape-swagger doesn`t evaluate procs, it shows stringified representation of proc, as in screenshot:

image

This pull request fixes it.

@grape-bot
Copy link

2 Warnings
⚠️ There're library changes, but not tests. That's OK as long as you're refactoring existing code.
⚠️ Unless you're refactoring existing code or improving documentation, please update CHANGELOG.md.

Here's an example of a CHANGELOG.md entry:

* [#959](https://github.com/ruby-grape/grape-swagger/pull/959): Add support for procs in default value - [@seriousdev-gh](https://github.com/seriousdev-gh).

Generated by 🚫 Danger

@numbata
Copy link
Contributor

numbata commented May 30, 2025

Hi @seriousdev-gh!

Thanks for adding Proc support for default values - I really appreciate the added flexibility! But I do have a small concern: executing arbitrary Procs at definition time could introduce unexpected performance or side-effect risks. For example, someone could write a Proc that:

  • Makes an HTTP call with no timeout.
  • Runs a synchronous DB query that sleeps for several seconds.
  • Writes to a log file.
  • Depends on something non-obvious.

Additionally, grape-swagger invokes the Proc outside of the usual Grape request context (see how DefaultValidator works here), which might lead to subtle differences in behavior or potential errors even.

Would it make sense to avoid immediately calling the Proc and provide some placeholder instead? For example, we can check do we have something non-proc-able in documentation section? Or simply omit the default field for Procs.

This way we preserve Proc support for real requests, but avoid any risks during doc generation.

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.

3 participants