Skip to content

Commit

Permalink
always include path, name and code on S3Errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@cirospaciari
cirospaciari committed Dec 31, 2024
1 parent f05fb4d commit 4cbee12
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 15 deletions.
3 changes: 0 additions & 3 deletions src/bun.js/bindings/ErrorCode.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,4 @@ export default [
["ERR_AWS_INVALID_PATH", Error],
["ERR_AWS_INVALID_ENDPOINT", Error],
["ERR_AWS_INVALID_SIGNATURE", Error],

// S3
["ERR_S3_FILE_NOT_FOUND", Error],
] as ErrorCodeMapping;
4 changes: 2 additions & 2 deletions src/bun.js/webcore/S3File.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ pub const S3BlobStatTask = struct {
this.promise.resolve(globalThis, .true);
},
.failure => |err| {
this.promise.rejectOnNextTick(globalThis, err.toJS(globalThis));
this.promise.rejectOnNextTick(globalThis, err.toJS(globalThis, this.store.data.s3.path()));
},
}
}
Expand All @@ -347,7 +347,7 @@ pub const S3BlobStatTask = struct {
this.promise.resolve(globalThis, JSValue.jsNumber(stat.size));
},
.failure => |err| {
this.promise.rejectOnNextTick(globalThis, err.toJS(globalThis));
this.promise.rejectOnNextTick(globalThis, err.toJS(globalThis, this.store.data.s3.path()));
},
}
}
Expand Down
13 changes: 9 additions & 4 deletions src/bun.js/webcore/blob.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -905,14 +905,15 @@ pub const Blob = struct {

const Wrapper = struct {
promise: JSC.JSPromise.Strong,
store: *Store,
pub usingnamespace bun.New(@This());

pub fn resolve(result: AWS.S3UploadResult, this: *@This()) void {
if (this.promise.globalObject()) |globalObject| {
switch (result) {
.success => this.promise.resolve(globalObject, JSC.jsNumber(0)),
.failure => |err| {
this.promise.rejectOnNextTick(globalObject, err.toJS(globalObject));
this.promise.rejectOnNextTick(globalObject, err.toJS(globalObject, this.store.data.s3.path()));
},
}
}
Expand All @@ -921,15 +922,19 @@ pub const Blob = struct {

fn deinit(this: *@This()) void {
this.promise.deinit();
this.store.deref();
this.destroy();
}
};

const promise = JSC.JSPromise.Strong.init(ctx);
const promise_value = promise.value();
const proxy = ctx.bunVM().transpiler.env.getHttpProxy(true, null);
const proxy_url = if (proxy) |p| p.href else null;
destination_blob.store.?.ref();
aws_options.credentials.s3Upload(s3.path(), "", destination_blob.contentTypeOrMimeType(), proxy_url, @ptrCast(&Wrapper.resolve), Wrapper.new(.{
.promise = promise,
.store = destination_blob.store.?,
}));
return promise_value;
}
Expand Down Expand Up @@ -1061,7 +1066,7 @@ pub const Blob = struct {
switch (result) {
.success => this.promise.resolve(globalObject, JSC.jsNumber(this.store.data.bytes.len)),
.failure => |err| {
this.promise.rejectOnNextTick(globalObject, err.toJS(globalObject));
this.promise.rejectOnNextTick(globalObject, err.toJS(globalObject, this.store.data.s3.path()));
},
}
}
Expand Down Expand Up @@ -3487,7 +3492,7 @@ pub const Blob = struct {
self.promise.reject(globalObject, bun.S3.createNotFoundError(globalObject, self.store.data.s3.path()));
},
.failure => |err| {
self.promise.rejectOnNextTick(globalObject, err.toJS(globalObject));
self.promise.rejectOnNextTick(globalObject, err.toJS(globalObject, self.store.data.s3.path()));
},
}
}
Expand Down Expand Up @@ -3841,7 +3846,7 @@ pub const Blob = struct {
this.promise.reject(this.globalThis, bun.S3.createNotFoundError(this.globalThis, this.blob.store.?.data.s3.path()));
},
.failure => |err| {
this.promise.rejectOnNextTick(this.globalThis, err.toJS(this.globalThis));
this.promise.rejectOnNextTick(this.globalThis, err.toJS(this.globalThis, this.blob.store.?.data.s3.path()));
},
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/bun.js/webcore/streams.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2676,6 +2676,16 @@ pub const FetchTaskletChunkedRequestSink = struct {
AutoFlusher.registerDeferredMicrotaskWithTypeUnchecked(@This(), this, this.globalThis.bunVM());
}

pub fn path(this: *@This()) ?[]const u8 {
if (this.task) |task| {
return switch (task) {
.s3_upload => |s3| s3.path,
else => null,
};
}
return null;
}

pub fn onAutoFlush(this: *@This()) bool {
if (this.done) {
this.auto_flusher.registered = false;
Expand Down
25 changes: 19 additions & 6 deletions src/s3.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -655,10 +655,14 @@ pub const AWSCredentials = struct {
code: []const u8,
message: []const u8,

pub fn toJS(err: *const @This(), globalObject: *JSC.JSGlobalObject) JSC.JSValue {
pub fn toJS(err: *const @This(), globalObject: *JSC.JSGlobalObject, path: ?[]const u8) JSC.JSValue {
//TODO: cache the structure of the error
const js_err = globalObject.createErrorInstance("{s}", .{err.message});
js_err.put(globalObject, JSC.ZigString.static("code"), JSC.ZigString.init(err.code).toJS(globalObject));
js_err.put(globalObject, JSC.ZigString.static("name"), JSC.ZigString.static("S3Error").toJS(globalObject));
if (path) |p| {
js_err.put(globalObject, JSC.ZigString.static("path"), JSC.ZigString.init(p).withEncoding().toJS(globalObject));
}
return js_err;
}
};
Expand Down Expand Up @@ -1054,7 +1058,7 @@ pub const AWSCredentials = struct {
}
if (state.status_code == 404) {
if (!has_body_code) {
code = "FileNotFound";
code = "NoSuchKey";
}
if (!has_body_message) {
message = "File not found";
Expand Down Expand Up @@ -1402,12 +1406,14 @@ pub const AWSCredentials = struct {
.ptr = .{ .Bytes = &reader.context },
.value = readable_value,
}, globalThis),
.path = bun.default_allocator.dupe(u8, path) catch bun.outOfMemory(),
}));
return readable_value;
}

const S3DownloadStreamWrapper = struct {
readable_stream_ref: JSC.WebCore.ReadableStream.Strong,
path: []const u8,
pub usingnamespace bun.New(@This());

pub fn callback(chunk: bun.MutableString, has_more: bool, request_err: ?S3Error, this: *@This()) void {
Expand All @@ -1422,7 +1428,7 @@ pub const AWSCredentials = struct {

readable.ptr.Bytes.onData(
.{
.err = .{ .JSValue = err.toJS(globalThis) },
.err = .{ .JSValue = err.toJS(globalThis, this.path) },
},
bun.default_allocator,
);
Expand Down Expand Up @@ -1455,6 +1461,7 @@ pub const AWSCredentials = struct {

pub fn deinit(this: *@This()) void {
this.readable_stream_ref.deinit();
bun.default_allocator.free(this.path);
this.destroy();
}
};
Expand Down Expand Up @@ -1484,6 +1491,7 @@ pub const AWSCredentials = struct {
callback: ?*const fn (S3UploadResult, *anyopaque) void,
callback_context: *anyopaque,
ref_count: u32 = 1,
path: []const u8, // this is owned by the task not by the wrapper
pub usingnamespace bun.NewRefCounted(@This(), @This().deinit);
pub fn resolve(result: S3UploadResult, self: *@This()) void {
const sink = self.sink;
Expand All @@ -1497,7 +1505,7 @@ pub const AWSCredentials = struct {
sink.abort();
return;
}
sink.endPromise.rejectOnNextTick(globalObject, err.toJS(globalObject));
sink.endPromise.rejectOnNextTick(globalObject, err.toJS(globalObject, self.path));
},
}
}
Expand Down Expand Up @@ -1600,6 +1608,7 @@ pub const AWSCredentials = struct {
.sink = &response_stream.sink,
.callback = callback,
.callback_context = callback_context,
.path = task.path,
});
task.callback_context = @ptrCast(ctx);
var signal = &response_stream.sink.signal;
Expand Down Expand Up @@ -1703,7 +1712,7 @@ pub const AWSCredentials = struct {
return;
}

sink.endPromise.rejectOnNextTick(globalObject, err.toJS(globalObject));
sink.endPromise.rejectOnNextTick(globalObject, err.toJS(globalObject, sink.path()));
},
}
}
Expand Down Expand Up @@ -2294,8 +2303,12 @@ pub const MultiPartUpload = struct {
}
};
pub fn createNotFoundError(globalThis: *JSC.JSGlobalObject, path: []const u8) JSC.JSValue {
//TODO: cache the structure of the error
const js_err = globalThis
.ERR_S3_FILE_NOT_FOUND("File {} not found", .{bun.fmt.quote(path)}).toJS();
.createErrorInstance("File {} not found", .{bun.fmt.quote(path)}).toJS(globalThis);
// make it consistent with S3 services
js_err.put(globalThis, JSC.ZigString.static("code"), JSC.ZigString.static("NoSuchKey").toJS(globalThis));
js_err.put(globalThis, JSC.ZigString.static("name"), JSC.ZigString.static("S3Error").toJS(globalThis));
js_err.put(globalThis, JSC.ZigString.static("path"), JSC.ZigString.init(path).withEncoding().toJS(globalThis));
return js_err;
}
2 changes: 2 additions & 0 deletions test/js/bun/s3/s3.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,8 @@ describe.skipIf(!s3Options.accessKeyId)("s3", () => {
expect.unreachable();
} catch (e: any) {
expect(e?.code).toBe("NoSuchKey");
expect(e?.path).toBe("do-not-exist.txt");
expect(e?.name).toBe("S3Error");
}
});
});
Expand Down

0 comments on commit 4cbee12

Please sign in to comment.