Skip to content

Conversation

sebsto
Copy link
Contributor

@sebsto sebsto commented Mar 14, 2025

This is a proposal to fix issue #507

changes

  • LambdaRuntime.init() uses a Mutex<Bool> to make sure only one instance is created
  • LambdaRuntime.init() can now throw an error in case an instance already exists (I did not use fatalError() to make it easier to test)
  • All convenience init() methods catch possible errors instead of re-throwing it to a void breaking the user-facing API
  • Renamed existing LambdaRuntimeError to LambdaRuntimeClientError
  • Introduced a new type LambdaRuntimeError to represent the double initialization error

@sebsto sebsto added kind/bug Feature doesn't work as expected. 🔨 semver/patch No public API change. labels Mar 14, 2025
@sebsto sebsto added this to the 2.0 milestone Mar 14, 2025
@sebsto sebsto requested a review from fabianfett March 14, 2025 12:54
@sebsto sebsto self-assigned this Mar 14, 2025
@sebsto
Copy link
Contributor Author

sebsto commented Mar 14, 2025

Compiler crashes on nightly build has been reported here
swiftlang/swift#80020

@sebsto
Copy link
Contributor Author

sebsto commented Mar 15, 2025

CI is now green, except API Breakage, which is expected.

@sebsto sebsto changed the title [core] Make LambdaRuntime a singleton without breaking the API (fix #507) [core] Make LambdaRuntime a singleton (fix #507) Mar 16, 2025
@sebsto sebsto added ⚠️ semver/major Breaks existing public API. and removed 🔨 semver/patch No public API change. labels Mar 16, 2025
Copy link
Member

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

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

Great spot! Thanks for tackling this one.

@sebsto
Copy link
Contributor Author

sebsto commented Mar 19, 2025

@fabianfett
I changed

  • Mutex for Atomic
  • Test is now on run() and not on init()
  • LamabdaRuntimeError returned and is now public + one additional error case
  • unit test is adjusted and pass

@sebsto sebsto requested a review from fabianfett March 19, 2025 19:50
@sebsto
Copy link
Contributor Author

sebsto commented Jun 29, 2025

Now that managing cancellation is working (thank you @adam b2811a5)

This PR is ready to review before merged.
API Breakage is expected as we are adding an error case.

@sebsto sebsto requested review from fabianfett and adam-fowler June 29, 2025 12:37
Copy link
Member

@adam-fowler adam-fowler left a comment

Choose a reason for hiding this comment

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

Looks good, only a couple of minor issues

@sebsto sebsto enabled auto-merge (squash) July 4, 2025 13:53
@sebsto sebsto merged commit 344d30b into swift-server:main Jul 4, 2025
32 checks passed
@sebsto sebsto deleted the sebsto/fix_507 branch July 4, 2025 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Feature doesn't work as expected. ⚠️ semver/major Breaks existing public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants