Skip to content

fix dynamodb lock table creation #1992

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

Merged
merged 1 commit into from
Jun 24, 2025

Conversation

nigelzor
Copy link
Contributor

@nigelzor nigelzor commented Jun 24, 2025

When trying to set up backendless Digger in a new AWS account I was blocked by errors in locking:

level=ERROR msg="Error describing DynamoDB table" tableName=DiggerDynamoDBLockTable error="operation error DynamoDB: DescribeTable, https response error StatusCode: 400, RequestID: D3AC..., ResourceNotFoundException: Requested resource not found: Table: DiggerDynamoDBLockTable not found"

level=ERROR msg="Failed to get DynamoDB item for lock" lockId=... error="operation error DynamoDB: GetItem, https response error StatusCode: 400, RequestID: 5BQQ..., ResourceNotFoundException: Requested resource not found"

Per the AWS docs1, DescribeTable throws ResourceNotFoundException, not TableNotFoundException. Checking for the wrong type caused createTableIfNotExists to always fail, though nothing was checking for the error it returned.

All callers now bubble up the error returned by createTableIfNotExists and there's no longer a hidden os.Exit(1) error handler.

With these changes I was able to run Digger successfully.

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling for DynamoDB table existence and creation, providing clearer error messages and preventing abrupt process termination.
    • Errors during table creation are now properly logged and returned, ensuring lock operations only proceed when the table is ready.
  • Tests

    • Updated tests to reflect improved error handling and table status checks for DynamoDB operations.

Footnotes

  1. https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_DescribeTable.html

per the AWS docs[^1], DescribeTable throws ResourceNotFoundException,
not TableNotFoundException. checking for the wrong type caused
createTableIfNotExists to always fail, although nothing was checking
for the error it returned.

[^1]: https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_DescribeTable.html
Copy link
Contributor

coderabbitai bot commented Jun 24, 2025

Walkthrough

The changes update error handling in DynamoDB table existence and creation logic. Error detection now uses ResourceNotFoundException instead of TableNotFoundException, and process termination on table creation failure is replaced with error returns. Callers now explicitly handle errors from table creation, and related tests are updated accordingly.

Changes

File(s) Change Summary
libs/locking/aws/dynamo_locking.go Switched error detection to ResourceNotFoundException, refactored error handling to return errors instead of exiting, updated method and variable names, and enforced explicit error handling in lock operations.
libs/locking/aws/dynamo_locking_test.go Updated mocks to use ResourceNotFoundException, adjusted table status reporting, and removed redundant mock calls in tests.

Sequence Diagram(s)

Loading
sequenceDiagram
    participant Caller
    participant DynamoDbLock
    participant DynamoDB

    Caller->>DynamoDbLock: Lock / Unlock / GetLock
    DynamoDbLock->>DynamoDbLock: createTableIfNotExists()
    DynamoDbLock->>DynamoDB: DescribeTable
    alt Table not found
        DynamoDbLock->>DynamoDB: CreateTable
        DynamoDbLock->>DynamoDB: WaitUntilTableCreated
        DynamoDbLock-->>Caller: Return error if creation fails
    else Table exists
        DynamoDbLock->>DynamoDB: Proceed with lock operation
    end
    DynamoDbLock-->>Caller: Return result or error

Possibly related PRs

  • fix e2e tests #1985: Modifies error handling in createTableIfNotExists in the same file, focusing on DynamoDB table existence errors and error blocking during table creation.

Poem

A table may hide or not be found,
But now our code stands on solid ground.
No more abrupt exits, just errors returned,
Each lock and unlock, carefully discerned.
With Dynamo's quirks now tamed at last,
Our rabbit hops forward—problems surpassed!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 474d3d0 and 253daad.

📒 Files selected for processing (2)
  • libs/locking/aws/dynamo_locking.go (8 hunks)
  • libs/locking/aws/dynamo_locking_test.go (1 hunks)
🔇 Additional comments (11)
libs/locking/aws/dynamo_locking_test.go (2)

23-23: Correctly updated to match AWS API documentation.

The change from TableNotFoundException to ResourceNotFoundException aligns with AWS documentation for the DescribeTable API and matches the corresponding production code fix.


26-26: Good addition of TableStatus field.

Adding the TableStatus: "ACTIVE" field ensures the mock properly simulates the expected DynamoDB table response structure that the production code now relies on.

libs/locking/aws/dynamo_locking.go (9)

39-48: Excellent fix - now uses the correct AWS exception type.

The function rename and exception type change from TableNotFoundException to ResourceNotFoundException aligns with AWS documentation for the DescribeTable API. This addresses the root cause where table creation was failing silently due to incorrect error detection.


58-58: Correct usage of updated exception checker.

The call to isResourceNotFoundException properly uses the updated error detection function.


72-72: Consistent error handling throughout the function.

Good consistency in using the updated isResourceNotFoundException function throughout the waiting logic.


78-82: Much improved error handling - no more process termination.

Replacing os.Exit(1) with proper error return is a significant improvement. This allows callers to handle table creation failures gracefully instead of abruptly terminating the entire process.


97-97: Consistent use of correct exception type.

The createTableIfNotExists function now correctly uses isResourceNotFoundException for error detection.


151-155: Proper error propagation in Lock method.

The Lock method now correctly handles and propagates errors from createTableIfNotExists. This ensures that table creation failures are surfaced to callers instead of being silently ignored.


219-223: Consistent error handling in Unlock method.

The Unlock method follows the same improved error handling pattern as the Lock method, ensuring consistent behavior across all DynamoDB operations.


233-233: Clean variable assignment without redeclaration.

Good practice to avoid variable redeclaration by using assignment instead of short variable declaration.


248-252: Complete error handling coverage.

The GetLock method now also properly handles errors from createTableIfNotExists, ensuring all methods consistently handle table creation failures.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate Unit Tests
  • Create PR with Unit Tests
  • Post Copyable Unit Tests in Comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Fixed critical DynamoDB lock table creation in backendless mode by correctly handling AWS API errors and improving error propagation.

  • Corrected error type check in libs/locking/aws/dynamo_locking.go from TableNotFoundException to ResourceNotFoundException to match AWS API behavior
  • Removed unsafe os.Exit(1) call from error handling path in dynamo_locking.go, properly bubbling up errors instead
  • Added proper table creation checks in Lock(), Unlock(), and GetLock() methods
  • Added TableStatus to mock implementation in dynamo_locking_test.go for more accurate testing

2 files reviewed, 1 comment
Edit PR Review Bot Settings | Greptile

@motatoes
Copy link
Contributor

Thank you! @nigelzor!

@motatoes motatoes merged commit 2dec859 into diggerhq:develop Jun 24, 2025
7 of 8 checks passed
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