LittleBigRefresh / Refresh

A second-generation custom server for LittleBigPlanet that focuses on quality of life features and improving user experience.
https://lbp.littlebigrefresh.com
GNU Affero General Public License v3.0
74 stars 25 forks source link

API bindings for other languages #221

Open jvyden opened 12 months ago

jvyden commented 12 months ago

We should encourage use of the API by writing some bindings for APIv3 in other languages.

These are the languages I'm most comfortable working in, but others can feel free to request others.

Most of these already have an existing partial implementation in one of our projects apart from Python and TypeScript, so we'd just be ripping those implementations out and exposing them as packages.

In general, API errors should be handled like exceptions, ApiResponse should never be returned directly, just the T. List responses should return a tuple/record of (T, ListInfo)

Beyley commented 12 months ago

I already have incomplete coverage of APIv3 inside of FreshPresence, this file can be pulled out and used as the basis for Zig bindings, although one thing to consider is Zig errors vs an ApiResponse wrapper type. Since a zig error has no payload, we wouldnt be able to include the API message when an error happens, this can be remedied in some ways, such as: you create a RefreshApi struct, and that struct has methods like GetLevels, which return an error union, but also if there is an error, it sets the message parameter on the original RefreshApi struct type eg.

const RefreshApi = struct {
    error_message: ?[]const u8 = null,

    allocator: std.mem.Allocator,

    const Error = ...;

    pub fn init(allocator: std.mem.Allocator) RefreshApi {
        return .{
            .allocator = allocator,
        };
    }

    pub fn getLevels(self: *RefreshApi, route: []const u8) Error!ApiList(Level) {
        var res: ApiResponse(ApiList(Level)) = doRequest();

        if(!res.success) {
            self.error_message = res.error.message;
            return apiErrorToZigError(res.error.name);
        }

        // toUnderlying here would free heap allocated things in ApiResponse, 
        // and leave only the underlying data structure [ApiList(Level) in this case] allocated, returning it
        return res.toUnderlying();
    }

    pub fn deinit(self: *RefreshApi) void {
        if(self.error_message) |error_message|      
            self.allocator.free(error_message);
    }
};

fn main() !void {
    var api = RefreshApi.init(allocator);
    defer api.deinit();

    //You can explicitly catch it to check the error message
    var levels: ApiList(Level) = api.getLevels("newest") catch |err| {
        std.debug.print("got err {s} with message {?s} while getting api levels", .{@errorName(err), api.error_message});
        return err;
    };

    //Or just use `try` and ignore the error message, only caring about the error type
    levels = try api.getLevels("newest");
}

I feel like this would be the best solution to the problem, while still having it feel idiomatic to zig, since you arent forced to use an external error union type, and can still use flat out normal try if you want

jvyden commented 12 months ago

Yeah, I think that's a good compromise. I don't really mind what you have to do as long as it's the right for that particular language.

One concern is the thread safety of that but obviously that's something you'd worry about during actual implementation

Beyley commented 12 months ago

Yeah, I think that's a good compromise. I don't really mind what you have to do as long as it's the right for that particular language.

One concern is the thread safety of that but obviously that's something you'd worry about during actual implementation

Thread safety isnt much of a concern, since Zig's HTTP client isnt thread safe in the first place, i think the best option here is to just document "hey this isnt thread safe, make sure to have your RefreshApi instance be threadlocal, or use std.Thread.Mutex", since trying to add thread safety on top would slow things down for single threaded cases (mutexes are slow), i think i'll pick up making this into a separate package at some point this week

jvyden commented 12 months ago

If the stdlib isn't thread-safe here then its probably fine to just warn people about it then