Repository navigation
fix: return underlying error in EnsureMigrationTable instead of swallowing it - #164
Merged
sinmetal merged 1 commit intoSep 29, 2026
Conversation
…owing it EnsureMigrationTable treated any read error as "table does not exist" and unconditionally attempted to create the migration table. This masked real failures such as permission errors or deadlines, since the CREATE TABLE call would then fail with a confusing or misleading error instead of the original one. Now only a NotFound error is treated as "table does not exist"; any other error is returned wrapped in the existing Error type. Fixes cloudspannerecosystem#114
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Contributor
Author
|
CLA signed. All checks ( |
Contributor
Author
Contributor
Author
sinmetal
self-requested a review
September 29, 2026 07:54
Collaborator
|
@TangoEnSkai Sorry for the delay in reviewing. Thank you for your contribution. |
sinmetal
approved these changes
Sep 29, 2026
2 of 3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
EnsureMigrationTableprobes for the migration table with aRead, andtreats any non-nil error from that read as "table does not exist,"
unconditionally falling through to
CREATE TABLE.What
Only treat a
codes.NotFounderror as "table does not exist." Any othererror (e.g. permission denied, deadline exceeded) is now returned wrapped
in the existing
Errortype instead of being swallowed.Why
If the read fails for a reason other than "table not found" (permission
error, timeout, etc.), wrench used to silently ignore that real error and
attempt to create the table anyway — which then fails with a confusing or
misleading error instead of surfacing the actual problem, exactly as
described in the issue.
Completion Criteria
go build ./...andgo vet ./...passTestEnsureMigrationTable(both "table already exists" and"table does not exist" cases) continues to pass the same behavior
close #114