acrisci / simple-breakpad-server

Simple breakpad crash reports collecting server
https://www.npmjs.com/package/simple-breakpad-server
MIT License
67 stars 29 forks source link

Add `extensions` map for downloaded files. #5

Closed Jimbly closed 7 years ago

Jimbly commented 7 years ago

Allows specifying the downloaded file's extension when the browser saves a file to disk. Especially useful for dump files.

acrisci commented 7 years ago

I can see this style of configuration getting out of hand quickly. What if we need to add more configuration to custom fields besides this?

As for the minidump, I think it should have a .dmp extension by default.

As for further configuration of custom fields and files, how about using an object for configuration?

files:
  - name: customfile
    fileName: customfile.txt

Then allow for the simpler configuration by string as well.

I'll have to think about the semantics of that, but it seems to be a better option given that more configuration might be in order.

Jimbly commented 7 years ago

Sounds like a reasonable idea.

Already I want to add a formatting function to configure how one of my custom fields is displayed (it's a Yaml block of a bunch of key/value pairs), though I'm not sure how that should be done - maybe the config should reference a node module/.js file which is require()'d to do the formatting, or could have some built-in formatters, but extendable is probably better, though too extendable lies the way of madness =).

Jimbly commented 7 years ago

Okay, I took a stab at this. customFields options are now all objects, I added some post-processing to the config to normalize things, and moved the ip and upload_file_minidump fields into the config object for less custom code in crashreport.coffee's SQL schema setup, and to apply a default downloadAs parameter for the minidump. If we want to add formatting methods here (maybe a Handlebars snippet would be a reasonable way to do that...) for the params, we could do that as well. I was thinking adding a formatter for IP would be useful (include a geoip link).

Though I've got tons of Node.js experience, this is the first time I've done anything on Coffeescript, so if there's anything that's got a cleaner way to do it in Coffeescript, feel free to let me know =)

Jimbly commented 7 years ago

Okay, I made the trivial change to the templating to at least look like Handlebars for now, shouldn't be too hard to add in something more complex if anyone has a need for it.

I think that's about it for changes to this branch. If you don't care about the history, I could rebase this down to a single commit before merging.