From 111a358454504203c2a0c323e4bcf77b030b0cb5 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Wed, 29 Apr 2026 13:30:09 +0200 Subject: [PATCH 1/3] filer: anchor notebook already-exists regex on RESOURCE_ALREADY_EXISTS The Workspace files import-file API used to return "Path () already exists." for notebook conflicts. The body now looks like: {"message":"Request failed for POST /Users/.../sqlNb.sql: RESOURCE_ALREADY_EXISTS: /Users/.../sqlNb already exists. Please pass overwrite=true to overwrite it."} There is no error_code field in the JSON, so the SDK leaves aerr.ErrorCode empty and errors.Is(aerr, ErrResourceAlreadyExists) does not fire. The same body shape is returned by aws/azure/gcp UCWS. Anchor the regex on the RESOURCE_ALREADY_EXISTS marker (the API's error code, embedded in the message) and capture the conflicting path that follows it. More stable than matching the human-readable "already exists. Please pass overwrite=true ..." sentence. This fixes TestImportDirDoesNotOverwrite and 8 TestFilerWorkspaceNotebook subtests on every cloud. Co-authored-by: Isaac --- libs/filer/workspace_files_client.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libs/filer/workspace_files_client.go b/libs/filer/workspace_files_client.go index c6c62816bbc..4edb66dcafa 100644 --- a/libs/filer/workspace_files_client.go +++ b/libs/filer/workspace_files_client.go @@ -209,8 +209,10 @@ func (w *WorkspaceFilesClient) Write(ctx context.Context, name string, reader io return fileAlreadyExistsError{absPath} } - // This API returns 400 if the file already exists, when the object type is notebook - regex := regexp.MustCompile(`Path \((.*)\) already exists.`) + // This API returns 400 if the file already exists, when the object type is notebook. + // The error_code field is empty in the JSON body, so we anchor on the + // RESOURCE_ALREADY_EXISTS marker that the API embeds in the message instead. + regex := regexp.MustCompile(`RESOURCE_ALREADY_EXISTS: (\S+)`) if aerr.StatusCode == http.StatusBadRequest && regex.MatchString(aerr.Message) { // Parse file path from regex capture group matches := regex.FindStringSubmatch(aerr.Message) From 18e9322ecc7c3d8f487c422b8f774025212ba301 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 4 May 2026 16:27:49 +0200 Subject: [PATCH 2/3] filer: keep both old and new already-exists regexes Per @pietern's feedback: we don't know how widely the new "RESOURCE_ALREADY_EXISTS: " format has been rolled out. Match both formats so the filer keeps working on workspaces still emitting the historical "Path () already exists." message. Co-authored-by: Isaac --- libs/filer/workspace_files_client.go | 33 ++++++++++++++++++---------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/libs/filer/workspace_files_client.go b/libs/filer/workspace_files_client.go index 4edb66dcafa..a6c63a6c22a 100644 --- a/libs/filer/workspace_files_client.go +++ b/libs/filer/workspace_files_client.go @@ -210,18 +210,29 @@ func (w *WorkspaceFilesClient) Write(ctx context.Context, name string, reader io } // This API returns 400 if the file already exists, when the object type is notebook. - // The error_code field is empty in the JSON body, so we anchor on the - // RESOURCE_ALREADY_EXISTS marker that the API embeds in the message instead. - regex := regexp.MustCompile(`RESOURCE_ALREADY_EXISTS: (\S+)`) - if aerr.StatusCode == http.StatusBadRequest && regex.MatchString(aerr.Message) { - // Parse file path from regex capture group - matches := regex.FindStringSubmatch(aerr.Message) - if len(matches) == 2 { - return fileAlreadyExistsError{matches[1]} - } + // We match both the historical "Path () already exists." format and the newer + // "RESOURCE_ALREADY_EXISTS: ..." format because the new format has not been + // rolled out to all workspaces yet. The error_code field is empty in the JSON body + // for both formats, so we anchor on markers in the message rather than using + // errors.Is(apierr.ErrResourceAlreadyExists). + notebookExistsRegexes := []*regexp.Regexp{ + regexp.MustCompile(`Path \((.*)\) already exists\.`), + regexp.MustCompile(`RESOURCE_ALREADY_EXISTS: (\S+)`), + } + if aerr.StatusCode == http.StatusBadRequest { + for _, regex := range notebookExistsRegexes { + if !regex.MatchString(aerr.Message) { + continue + } + // Parse file path from regex capture group + matches := regex.FindStringSubmatch(aerr.Message) + if len(matches) == 2 { + return fileAlreadyExistsError{matches[1]} + } - // Default to path specified to filer.Write if regex capture fails - return fileAlreadyExistsError{absPath} + // Default to path specified to filer.Write if regex capture fails + return fileAlreadyExistsError{absPath} + } } // This API returns StatusForbidden when you have read access but don't have write access to a file From a8f158609686e36f0e9fccdab2f3861a08b4423b Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 4 May 2026 16:55:24 +0200 Subject: [PATCH 3/3] filer: simplify already-exists detection to substring match on shared marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both message formats end with "already exists.", and absPath is what the caller asked to write — no need to parse the message. Co-authored-by: Isaac --- integration/libs/filer/filer_test.go | 2 +- libs/filer/workspace_files_client.go | 33 +++++++--------------------- 2 files changed, 9 insertions(+), 26 deletions(-) diff --git a/integration/libs/filer/filer_test.go b/integration/libs/filer/filer_test.go index 38cdf3c4060..58618628b7e 100644 --- a/integration/libs/filer/filer_test.go +++ b/integration/libs/filer/filer_test.go @@ -443,7 +443,7 @@ func TestFilerWorkspaceNotebook(t *testing.T) { // Assert uploading a second time fails due to overwrite mode missing err = f.Write(ctx, tc.name, strings.NewReader(tc.content2)) require.ErrorIs(t, err, fs.ErrExist) - assert.Regexp(t, `file already exists: .*/`+tc.nameWithoutExt+`$`, err.Error()) + assert.Regexp(t, `file already exists: .*/`+tc.name+`$`, err.Error()) // Try uploading the notebook again with overwrite flag. This time it should succeed. err = f.Write(ctx, tc.name, strings.NewReader(tc.content2), filer.OverwriteIfExists) diff --git a/libs/filer/workspace_files_client.go b/libs/filer/workspace_files_client.go index a6c63a6c22a..54103f8158a 100644 --- a/libs/filer/workspace_files_client.go +++ b/libs/filer/workspace_files_client.go @@ -11,7 +11,6 @@ import ( "net/http" "net/url" "path" - "regexp" "slices" "strings" "time" @@ -209,30 +208,14 @@ func (w *WorkspaceFilesClient) Write(ctx context.Context, name string, reader io return fileAlreadyExistsError{absPath} } - // This API returns 400 if the file already exists, when the object type is notebook. - // We match both the historical "Path () already exists." format and the newer - // "RESOURCE_ALREADY_EXISTS: ..." format because the new format has not been - // rolled out to all workspaces yet. The error_code field is empty in the JSON body - // for both formats, so we anchor on markers in the message rather than using - // errors.Is(apierr.ErrResourceAlreadyExists). - notebookExistsRegexes := []*regexp.Regexp{ - regexp.MustCompile(`Path \((.*)\) already exists\.`), - regexp.MustCompile(`RESOURCE_ALREADY_EXISTS: (\S+)`), - } - if aerr.StatusCode == http.StatusBadRequest { - for _, regex := range notebookExistsRegexes { - if !regex.MatchString(aerr.Message) { - continue - } - // Parse file path from regex capture group - matches := regex.FindStringSubmatch(aerr.Message) - if len(matches) == 2 { - return fileAlreadyExistsError{matches[1]} - } - - // Default to path specified to filer.Write if regex capture fails - return fileAlreadyExistsError{absPath} - } + // This API returns 400 if the file already exists when the object type is notebook. + // Both the historical "Path () already exists." format and the newer + // "RESOURCE_ALREADY_EXISTS: already exists. ..." format end with the same + // "already exists." marker; the JSON error_code is empty in both. The new format + // might not have been rolled out to all workspaces yet, so we anchor on the shared + // marker and return absPath rather than parsing the message. + if aerr.StatusCode == http.StatusBadRequest && strings.Contains(aerr.Message, "already exists.") { + return fileAlreadyExistsError{absPath} } // This API returns StatusForbidden when you have read access but don't have write access to a file