Replace error message matching with structural error check in apps dev#5517
Merged
Conversation
Two sites branched on err.Error() content. cmd/apps/dev.go now matches the Apps API 404 via errors.Is with apierr.ErrNotFound, and the DBFS filer Read re-stats the path to detect directories instead of matching the SDK's error message. Co-authored-by: Isaac
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Collaborator
Integration test reportCommit: 824a88c
23 interesting tests: 13 SKIP, 7 KNOWN, 3 RECOVERED
Top 4 slowest tests (at least 2 minutes):
|
Co-authored-by: Isaac
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
3 tasks
pietern
reviewed
Jul 2, 2026
| }) | ||
| if err != nil { | ||
| if strings.Contains(err.Error(), "does not exist") || strings.Contains(err.Error(), "is deleted") { | ||
| if isAppNotFound(err) { |
Member
Author
There was a problem hiding this comment.
Done, inlined the check at the call site and dropped the helper (824a88c).
| // when the path is a directory, so re-stat to detect that case. | ||
| if info, serr := w.workspaceClient.Dbfs.GetStatusByPath(ctx, absPath); serr == nil && info.IsDir { | ||
| return nil, notAFile{absPath} | ||
| } |
Contributor
There was a problem hiding this comment.
This requires an additional API call.
DBFS is on its way out so I prefer not making any more changes to it.
Member
Author
There was a problem hiding this comment.
Makes sense, reverted the DBFS change (and its added stat call) entirely (824a88c). This PR is now apps-only.
Per review: DBFS is being deprecated so revert the dbfs_client change (and its added stat call), and inline the isAppNotFound helper in apps dev into the errors.Is check at the call site. Co-authored-by: Isaac
simonfaltum
changed the title
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
pietern
approved these changes
Jul 2, 2026
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
3 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.
Why
Found during a full-repo review of the CLI.
cmd/apps/dev.gobranched on the text oferr.Error()(matching "does not exist" and "is deleted") to detect a missing app, which silently breaks the moment the upstream wording changes.Changes
Before,
dev-remotecompared the error message string; now it uses a structural check that keeps working if the wording changes.cmd/apps/dev.go: the Apps API reports a missing or deleted app with HTTP 404 ("App with name X does not exist or is deleted."), sodev-remotenow checkserrors.Is(err, apierr.ErrNotFound)inline at the call site. Note the 404 carries error codeNOT_FOUND, which the SDK maps to a sentinel by status code only, soapierr.ErrResourceDoesNotExistwould not match here.The DBFS filer change from the original version of this PR was dropped: DBFS is being deprecated, and the fix there required an extra API call on the error path.
Test plan
go test ./cmd/appspasses../task fmt-q,./task lint-q, and./task checkspass.