Skip to content

Fix infinite recursion when the logger enters a loop#304

Merged
ArturT merged 15 commits intomainfrom
fix-stack-level-too-deep
Jul 14, 2025
Merged

Fix infinite recursion when the logger enters a loop#304
ArturT merged 15 commits intomainfrom
fix-stack-level-too-deep

Conversation

@ArturT
Copy link
Copy Markdown
Member

@ArturT ArturT commented Jul 10, 2025

Story

https://trello.com/c/yROeD8FX

Related

Description

Fix infinite recursion when the logger enters a loop. The issue occurs when KNAPSACK_PRO_LOG_DIR=log is set and a conflict arises between the values of environment variables from the CI provider and those configured by the user (e.g., KNAPSACK_PRO_CI_NODE_INDEX, KNAPSACK_PRO_CI_NODE_TOTAL).

Changes:

  • Log a warning message using warn instead of the Knapsack Pro logger to prevent infinite recursion.

Checklist reminder

  • You added the changes to the UNRELEASED section of the CHANGELOG.md, including the needed bump (ie, patch, minor, major)
  • You follow the architecture outlined below for RSpec in Queue Mode, which is a work in progress (feel free to propose changes):
    • Pure: lib/knapsack_pro/pure/queue/rspec_pure.rb contains pure functions that are unit tested.
    • Extension: lib/knapsack_pro/extensions/rspec_extension.rb encapsulates calls to RSpec internals and is integration and e2e tested.
    • Runner: lib/knapsack_pro/runners/queue/rspec_runner.rb invokes the pure code and the extension to produce side effects, which are integration and e2e tested.

@ArturT ArturT added the bug label Jul 10, 2025

if !knapsack_env_value.nil? && !ci_env_value.nil? && knapsack_env_value != ci_env_value.to_s
KnapsackPro.logger.info("You have set the environment variable #{knapsack_env_name} to #{knapsack_env_value} which could be automatically determined from the CI environment as #{ci_env_value}.")
warn("You have set the environment variable #{knapsack_env_name} to #{knapsack_env_value} which could be automatically determined from the CI environment as #{ci_env_value}.")
Copy link
Copy Markdown
Member Author

@ArturT ArturT Jul 11, 2025

Choose a reason for hiding this comment

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

Regardless of

Warning[:deprecated] = false

or

Warning[:deprecated] = true

the following warning is printed to stderr:

warn("You have set the environment variable #{knapsack_env_name} to #{knapsack_env_value} which could be automatically determined from the CI environment as #{ci_env_value}.")

This is fine. We expect it to be printed. After all, it's not a deprecation warning. It's a misconfiguration warning.

If we were to add the :deprecated category to the following message, then the suppression would be respected if Warning[:deprecated] = false:

# see the end of the line
warn("You have set the environment variable #{knapsack_env_name} to #{knapsack_env_value} which could be automatically determined from the CI environment as #{ci_env_value}.", category: :deprecated)

References:

@ArturT ArturT changed the title Fix infinite recursion when logger gets into the loop Fix infinite recursion when the logger gets into a loop Jul 11, 2025
Comment thread CHANGELOG.md Outdated

### 8.3.2

* Fix infinite recursion when the logger gets into a loop
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.

plea

Please add more information here. The issue is a good starting point.

@ArturT ArturT changed the title Fix infinite recursion when the logger gets into a loop Fix infinite recursion when the logger enters a loop Jul 14, 2025
@ArturT ArturT merged commit cbf4a53 into main Jul 14, 2025
25 checks passed
@ArturT ArturT deleted the fix-stack-level-too-deep branch July 14, 2025 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants