Request: API Change / Addition to sentry_capture_minidump

[zsd4yr]

RE: https://github.com/getsentry/sentry-native/pull/1067/files

Would it be possible to change / add new API here to make the result more manageable / informational?
Some ideas

• sentry_capture_minidump should have a return value indicating whether or not it succeeded
• perhaps the report event id would be good? Some information that can be used to lookup the actual dump

These even ids appear to be coming back when observed in fiddler, we just need some way to get ahold of them :)

[zsd4yr]

I was able to make this change and validate it locally through our sentry usage -- for whatever reason I am getting denied trying to push my changes into a branch in this repo

The changes are very simple if someone had any advise on how I can create a branch to PR them

I'll admit I'm not usually a fan of changing API surface area, but an additive change like this should be fine. If you're worried about back compatibility, we could always just make a second version of the methods too, sentry_capture_minidump_with_return or something
 
sentry.h

	/**
	 * Allows capturing independently created minidumps.
	 *
	 * This generates a fatal error event, includes the scope and attachments.
	 * If the event isn't dropped by a before-send hook, the minidump is attached
	 * and the event is sent.
	 *
	 * If returns a nil UUID, the minidump was not successfully captured. Otherwise,
	 * will return the UUID of the event processed with the minidump attachment.
	 */
	SENTRY_API sentry_uuid_t sentry_capture_minidump(const char* path);
	SENTRY_API sentry_uuid_t sentry_capture_minidump_n(const char* path, size_t path_len);

sentry_core.c

sentry_uuid_t
sentry_capture_minidump(const char *path)
{
    return sentry_capture_minidump_n(path, sentry__guarded_strlen(path));
}

sentry_uuid_t
sentry_capture_minidump_n(const char *path, size_t path_len)
{
    sentry_uuid_t eventId = sentry_uuid_nil();

    sentry_path_t *dump_path = sentry__path_from_str_n(path, path_len);

    if (!dump_path) {
        SENTRY_WARN(
            "sentry_capture_minidump() failed due to null path to minidump");
        return eventId;
    }

    SENTRY_DEBUGF(
        "Capturing minidump \"%" SENTRY_PATH_PRI "\"", dump_path->path);

    sentry_value_t event = sentry_value_new_event();
    sentry_value_set_by_key(
        event, "level", sentry__value_new_level(SENTRY_LEVEL_FATAL));

    SENTRY_WITH_OPTIONS (options) {
        sentry_envelope_t *envelope
            = sentry__prepare_event(options, event, NULL, true);

        if (envelope) {
            // the minidump is added as an attachment, with type
            // `event.minidump`
            sentry_envelope_item_t *item = sentry__envelope_add_from_path(
                envelope, dump_path, "attachment");
            if (item) {
                sentry__envelope_item_set_header(item, "attachment_type",
                    sentry_value_new_string("event.minidump"));

                sentry__envelope_item_set_header(item, "filename",
#ifdef SENTRY_PLATFORM_WINDOWS
                    sentry__value_new_string_from_wstr(
#else
                    sentry_value_new_string(
#endif
                        sentry__path_filename(dump_path)));
            }

            sentry__capture_envelope(options->transport, envelope);
            eventId = sentry__envelope_get_event_id(envelope);

            SENTRY_INFOF("Minidump has been captured: \"%" SENTRY_PATH_PRI "\"",
                dump_path->path);
        }
    }

    sentry__path_free(dump_path);

    return eventId;
}

[supervacuus]

Returning the event_id could be a valuable addition, and I would be willing to release a breaking release since it isn't a big break to a relatively new interface.

But I also want to be clear that returning an event_id does not mean the minidump was successfully stored in the backend. The transport runs asynchronously to the calling thread, and you will get a locally generated UUID and not one coming in response from the server.

So, if you want to store the IDs so you can look up your uploads later, this is the right approach. If you expect it as a response ticket for a successfully uploaded minidump, that is not what you get.

The changes are very simple if someone had any advise on how I can create a branch to PR them

You must fork the repository, push the branch to that fork, and then create a PR from the branch of your fork onto our master. Uploading branches directly to the repository is only available to maintainers.

[zsd4yr]

Hey @supervacuus thanks for the feedback and advise!

What do you think would need to be done to get the full response back from the server to return? It would need to wait around for something, yes?

I'll make a PR with this version as any kind of await pattern would likely be a different entrypoint method

[zsd4yr]

Lemme know if this is incorrect :) #1138

[supervacuus]

What do you think would need to be done to get the full response back from the server to return? It would need to wait around for something, yes?

I wouldn't return the complete response in the public API, but we could add a synchronous interface to transports to block the caller instead of enqueuing a send task. Then, we could provide synchronous capture functions in the public API.