Crash UI and manual uploads

[Gayane Petrosyan]

Hi crashpad-dev team,

I am looking at adding functionality to about::crashes to allow users manually upload unsent crashes ( crbug.com/620762 ). I was looking at crash reporter for windows for now. Here is the function that returns the list of uploads from Chrome Elf for crash UI and I imagine actual crash log sending part would be somewhere in third_party/crashpad/crashpad/

Could you point me where the decision is made to send logs or not? and what would be the best way for plugging this units together? 

If sending individual uploads will be too complicated we could also send all the unsent ones.

Thanks,

Gayane

[Mark Mentovai]

We’ll need something in crashpad::CrashReportDatabase to bump a “completed” un-uploaded report back to “pending”. This will need to happen for all of our database implementations, but we’ve only got two: mac and win. Chrome hangs on to one of these database objects in components/crash/content/app/crashpad.cc as g_database, so like crash_reporter::GetReports(), you’d need to hook into the new Crashpad code from there. The crash_upload_list_crashpad.cc file that you referenced is just one more layer up from that.

--
You received this message because you are subscribed to the Google Groups "Crashpad-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to crashpad-dev+unsubscribe@chromium.org.
To post to this group, send email to crashp...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/crashpad-dev/CAO-RN54jTs5-1S-Vy4Pxfn4UZkdO9SEBBSMmJfC39sShG_4fcQ%40mail.gmail.com.

[Mark Mentovai]

I gave the wrong links for the database implementations. They should be mac and win.

[Gayane Petrosyan]

Thanks for all the info. What would be the state of the reports in following cases:
 - when crash reporting is not enabled

 - crash reporting is enabled but no network connection

 

On Thu, Aug 4, 2016 at 2:35 PM Mark Mentovai <ma...@chromium.org> wrote:

I gave the wrong links for the database implementations. They should be mac and win.

On Thu, Aug 4, 2016 at 2:33 PM, Mark Mentovai <ma...@chromium.org> wrote:

We’ll need something in crashpad::CrashReportDatabase to bump a “completed” un-uploaded report back to “pending”. This will need to happen for all of our database implementations, but we’ve only got two: mac and win. Chrome hangs on to one of these database objects in components/crash/content/app/crashpad.cc as g_database, so like crash_reporter::GetReports(), you’d need to hook into the new Crashpad code from there. The crash_upload_list_crashpad.cc file that you referenced is just one more layer up from that.

On Thu, Aug 4, 2016 at 2:18 PM, 'Gayane Petrosyan' via Crashpad-dev <crashp...@chromium.org> wrote:

Hi crashpad-dev team,

I am looking at adding functionality to about::crashes to allow users manually upload unsent crashes ( crbug.com/620762 ). I was looking at crash reporter for windows for now. Here is the function that returns the list of uploads from Chrome Elf for crash UI and I imagine actual crash log sending part would be somewhere in third_party/crashpad/crashpad/

Could you point me where the decision is made to send logs or not? and what would be the best way for plugging this units together? 

If sending individual uploads will be too complicated we could also send all the unsent ones.

Thanks,

Gayane

--
You received this message because you are subscribed to the Google Groups "Crashpad-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to crashpad-dev...@chromium.org.

[Mark Mentovai]

Gayane Petrosyan wrote:

Thanks for all the info. What would be the state of the reports in following cases:
 - when crash reporting is not enabled

Completed, with Report::uploaded = false.

 - crash reporting is enabled but no network connection

Pending if it will be retried. Completed with Report::uploaded = false if the maximum number of retry attempts has been reached. Note, however, currently only one try will ever be attempted, although this it can be increased fairly easily.

I did think of an additional twist: even if we move a completed report back into pending, if uploads aren’t enabled, crashpad_handler will just kick it right back to completed-not-uploaded. Since one of the things that we do want to cover in bug 620762 is to upload individual reports even when overall reporting consent has been withheld, we’ll need a way to overcome that switch. We’re also talking about client-side sampling (bug 609987), and we discussed implementing this by telling Crashpad that uploads are disabled. For that, too, we’d want to be able to upload reports even when Crashpad’s view of the global upload switch is off. To cover this aspect of the problem, we could introduce a new “force” bit on the pending state. This could probably go into ReportState on Windows (avoiding major breaking changes to the database metadata file) and we could use an extended attribute on Mac (also avoiding a breaking change to the database structure).

However, there are cases where we really do want uploads to be disabled without the possibility of the client forcing an upload. Here’s the current logic in Chrome for what to tell Crashpad about uploads. If reporting is forced off by policy, we’d want to respect that and deny users the ability to force an upload manually. We could implement that with another bit in the Crashpad database settings, but then I think things are getting needlessly complex. We could also just say that this is all the client’s responsibility, and that if the client really doesn’t want to allow a user to force an upload, then it should avoid giving the user that option. The only thing that’s attractive about putting this knowledge into Crashpad is that it makes it harder for the client (Chrome) to do something stupid. But I don’t know if that alone is reason enough to put this into Crashpad. My gut says no.

As in yesterday’s thread, we’re talking here about making changes to the database from outside of crashpad_handler (here, we’ll be making the changes from the Chrome browser process). This is safe and we designed for it because we contemplated all of these use cases. We do need to keep in mind that currently, crashpad_handler only scans for database changes like this once every 15 minutes. We can and should make this respond to filesystem changes instead of being strictly on a timer. We haven’t done that yet because until yesterday and today, nobody was talking about making changes to the database from outside of crashpad_handler yet.

On Thu, Aug 4, 2016 at 2:35 PM Mark Mentovai <ma...@chromium.org> wrote:

I gave the wrong links for the database implementations. They should be mac and win.

On Thu, Aug 4, 2016 at 2:33 PM, Mark Mentovai <ma...@chromium.org> wrote:

We’ll need something in crashpad::CrashReportDatabase to bump a “completed” un-uploaded report back to “pending”. This will need to happen for all of our database implementations, but we’ve only got two: mac and win. Chrome hangs on to one of these database objects in components/crash/content/app/crashpad.cc as g_database, so like crash_reporter::GetReports(), you’d need to hook into the new Crashpad code from there. The crash_upload_list_crashpad.cc file that you referenced is just one more layer up from that.

On Thu, Aug 4, 2016 at 2:18 PM, 'Gayane Petrosyan' via Crashpad-dev <crashp...@chromium.org> wrote:

Hi crashpad-dev team,

I am looking at adding functionality to about::crashes to allow users manually upload unsent crashes ( crbug.com/620762 ). I was looking at crash reporter for windows for now. Here is the function that returns the list of uploads from Chrome Elf for crash UI and I imagine actual crash log sending part would be somewhere in third_party/crashpad/crashpad/

Could you point me where the decision is made to send logs or not? and what would be the best way for plugging this units together? 

If sending individual uploads will be too complicated we could also send all the unsent ones.

Thanks,

Gayane

--
You received this message because you are subscribed to the Google Groups "Crashpad-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to crashpad-dev+unsubscribe@chromium.org.

[Gayane Petrosyan]

Thanks a lot. 

I did think of an additional twist: even if we move a completed report back into pending, if uploads aren’t enabled, crashpad_handler will just kick it right back to completed-not-uploaded. Since one of the things that we do want to cover in bug 620762 is to upload individual reports even when overall reporting consent has been withheld, we’ll need a way to overcome that switch. We’re also talking about client-side sampling (bug 609987), and we discussed implementing this by telling Crashpad that uploads are disabled. For that, too, we’d want to be able to upload reports even when Crashpad’s view of the global upload switch is off. To cover this aspect of the problem, we could introduce a new “force” bit on the pending state. This could probably go into ReportState on Windows (avoiding major breaking changes to the database metadata file)

I agree that we would need a 4th report state for forcing uploads on Win.

 

and we could use an extended attribute on Mac (also avoiding a breaking change to the database structure).

As we are going to do crash reports sampling only on Win and Android we are interested implementing manual uploads for those platforms first. But I can have a look on Mac as well if there is significant overlap.

  

However, there are cases where we really do want uploads to be disabled without the possibility of the client forcing an upload. Here’s the current logic in Chrome for what to tell Crashpad about uploads. If reporting is forced off by policy, we’d want to respect that and deny users the ability to force an upload manually. We could implement that with another bit in the Crashpad database settings, but then I think things are getting needlessly complex.

Why would this need to go into database settings? What if we check it along with the forced report state before uploading the report?

We could also just say that this is all the client’s responsibility, and that if the client really doesn’t want to allow a user to force an upload, then it should avoid giving the user that option. The only thing that’s attractive about putting this knowledge into Crashpad is that it makes it harder for the client (Chrome) to do something stupid. But I don’t know if that alone is reason enough to put this into Crashpad. My gut says no.

I could add a check in Crash UI not to have manual uploads if there is a enforced policy if that sounds better.

 

As in yesterday’s thread, we’re talking here about making changes to the database from outside of crashpad_handler (here, we’ll be making the changes from the Chrome browser process). This is safe and we designed for it because we contemplated all of these use cases. We do need to keep in mind that currently, crashpad_handler only scans for database changes like this once every 15 minutes. We can and should make this respond to filesystem changes instead of being strictly on a timer. We haven’t done that yet because until yesterday and today, nobody was talking about making changes to the database from outside of crashpad_handler yet.

Do you think this is something that crashpad team will work on in near future.

[Mark Mentovai]

Gayane Petrosyan wrote:

and we could use an extended attribute on Mac (also avoiding a breaking change to the database structure).

As we are going to do crash reports sampling only on Win and Android we are interested implementing manual uploads for those platforms first. But I can have a look on Mac as well if there is significant overlap.

We still want to handle manual upload on Mac in the “no blanket consent, but user wants to upload a single report” and “automatic upload attempt(s) failed, user wants to attempt manual submission” cases. Note that bug 620762, which covers this feature, is targeted to Windows and Mac.

Crashpad basically aims for parity across supported platforms. If we do manual upload anywhere, I’m not going to let it only be a subset of platforms.

However, there are cases where we really do want uploads to be disabled without the possibility of the client forcing an upload. Here’s the current logic in Chrome for what to tell Crashpad about uploads. If reporting is forced off by policy, we’d want to respect that and deny users the ability to force an upload manually. We could implement that with another bit in the Crashpad database settings, but then I think things are getting needlessly complex.

Why would this need to go into database settings? What if we check it along with the forced report state before uploading the report?

Crashpad can’t read Chrome’s settings, so Chrome has to push information that Crashpad needs to make its decisions in to it. The current upload-enabled preference is one example of this. If, as I was saying, per-report manual upload is able to override the database setting’s upload-disabled setting, then we may need an even bigger upload-enabled switch that manual upload can’t override. This bigger switch would also go on the database settings, and it would be set to disable all uploads if policy forbids crash reporting. But I was also saying that we may not need this, if we can just make it Chrome’s problem and require Chrome to never ask Crashpad to do a manual upload when policy dictates “no crash reporting.”

As in yesterday’s thread, we’re talking here about making changes to the database from outside of crashpad_handler (here, we’ll be making the changes from the Chrome browser process). This is safe and we designed for it because we contemplated all of these use cases. We do need to keep in mind that currently, crashpad_handler only scans for database changes like this once every 15 minutes. We can and should make this respond to filesystem changes instead of being strictly on a timer. We haven’t done that yet because until yesterday and today, nobody was talking about making changes to the database from outside of crashpad_handler yet.

Do you think this is something that crashpad team will work on in near future.

I don’t know. It seems like it might be necessary for both of these features.