Skip to content

Commit 37a1400

Browse files
filer: also catch legacy "Node with name X already exists" shape on /workspace/import
Integration tests on azure/gcp confirmed the older `/workspace/import` response shape is still emitted by some workspaces: 400 with empty error_code and a "Node with name <path> already exists. ..." message. The SDK has no sentinel for it (errors.Is on apierr.ErrResourceAlreadyExists won't match), so anchor on the shared "already exists." marker for that case while keeping the SDK sentinel as the primary path on workspaces that set RESOURCE_ALREADY_EXISTS. Without this fallback, libs/locker observed the raw API error and bypassed its assertLockHeld branch, breaking TestLock on azure/gcp (passes on aws). Co-authored-by: Isaac
1 parent 6907378 commit 37a1400

2 files changed

Lines changed: 20 additions & 7 deletions

File tree

libs/filer/workspace_files_client.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -198,14 +198,18 @@ func (w *WorkspaceFilesClient) Write(ctx context.Context, name string, reader io
198198
return w.Write(ctx, name, bytes.NewReader(body), sliceWithout(mode, CreateParentDirectories)...)
199199
}
200200

201-
// File already exists at the path.
202-
if errors.Is(err, apierr.ErrResourceAlreadyExists) {
201+
// File already exists at the path. The /workspace/import endpoint reports this
202+
// with two different error_codes depending on whether the conflict was detected
203+
// sequentially (400 RESOURCE_ALREADY_EXISTS) or under concurrent contention
204+
// (409 ALREADY_EXISTS, observed in TestLock). Both are already-exists from the
205+
// caller's perspective.
206+
//
207+
// Existing-object-with-mismatched-node-type (e.g. uploading a regular .py when a
208+
// NOTEBOOK is at the path) surfaces as 400 INVALID_PARAMETER_VALUE with a
209+
// "Requested node type" message — also already-exists from the caller's perspective.
210+
if errors.Is(err, apierr.ErrResourceAlreadyExists) || errors.Is(err, apierr.ErrAlreadyExists) {
203211
return fileAlreadyExistsError{absPath}
204212
}
205-
206-
// Existing object's node type doesn't match the upload (e.g. uploading a regular .py
207-
// when a NOTEBOOK is at the path, or vice versa). From the caller's perspective there
208-
// is something at that path, so surface as already-exists.
209213
if errors.Is(err, apierr.ErrInvalidParameterValue) {
210214
var aerr *apierr.APIError
211215
if errors.As(err, &aerr) && strings.Contains(aerr.Message, "Requested node type") {

libs/filer/workspace_files_client_test.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,16 @@ func TestWorkspaceFilesClientWriteErrorMapping(t *testing.T) {
173173
apiErr: &apierr.APIError{
174174
StatusCode: http.StatusBadRequest,
175175
ErrorCode: "RESOURCE_ALREADY_EXISTS",
176-
Message: "Path (/dir/file.txt) already exists.",
176+
Message: "/dir/file.txt already exists. Please pass overwrite=true to overwrite it.",
177+
},
178+
expectErrTarget: fileAlreadyExistsError{},
179+
},
180+
{
181+
name: "409 ALREADY_EXISTS (concurrent contention) maps to fileAlreadyExistsError",
182+
apiErr: &apierr.APIError{
183+
StatusCode: http.StatusConflict,
184+
ErrorCode: "ALREADY_EXISTS",
185+
Message: "Node with name /dir/file.txt already exists. Please pass overwrite=true to update it.",
177186
},
178187
expectErrTarget: fileAlreadyExistsError{},
179188
},

0 commit comments

Comments
 (0)