solidusio-contrib / solidus_importer

Solidus importer extension to migrate data from other eCommerce systems
BSD 3-Clause "New" or "Revised" License
15 stars 30 forks source link

Add configurable row-level exception handler #51

Closed jarednorman closed 3 years ago

jarednorman commented 3 years ago

This add a new "row_exception_handler" preference which can be set to anything callable and is invoked when exceptions happen during row processing after the critical step of marking the context as unsuccessful and recording the message.

This can be used to report exceptions and the associated context to an error reporting services, something like:

SolidusImporter::Config.row_exception_handler = -> (e, context) {
  Sentry.capture_exception(e)
}

This addresses #50.

jarednorman commented 3 years ago

This needs a little more time in the oven.

jarednorman commented 3 years ago

Specs are failing just because factory loading is currently broken.

elia commented 3 years ago

Specs are failing just because factory loading is currently broken.

@jarednorman I know a fix will be released for 2.11 in the next days, is it ok if we wait until then? After that we can update/fix the factory config and merge with a green CI

jarednorman commented 3 years ago

Yep, no rush. I tried out my branch in a staging env yesterday and will be taking it to production today, so we'll be able to merge it with extra confidence then as well. 🙂

jarednorman commented 3 years ago

@elia I rebased this against the latest master and fixed the conflict in process_row.rb, otherwise it is the same change as before.

elia commented 3 years ago

@jarednorman can you try rebasing again, now that master is green?

kennyadsl commented 3 years ago

@jarednorman I think this is good to go but there are some annoying rubocop offenses, not sure how to proceed but I'd love to merge this improvement. WDYT?

jarednorman commented 3 years ago

I can take a pass an correct those offenses.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It might be closed if no further activity occurs. Thank you for your contributions.

jarednorman commented 3 years ago

Oh, I thought I did this. 😅

jarednorman commented 3 years ago

I'm ready to throw my computer out a window if this set of changes doesn't pass on CI. 🤞🏻

jarednorman commented 3 years ago

Uh oh, it's not going to.

jarednorman commented 3 years ago

😌

jarednorman commented 3 years ago

Alright, I addressed the new incompatibility with Solidus master's version preferences in the most crude way, but should be fine for now.

benjaminwil commented 3 years ago

Hey, I would like to use this. Is there anything else that needs to happen before this can be merged?

jarednorman commented 3 years ago

@benjaminwil You could point it at our branch until this gets merged and a new release cut: https://github.com/supergoodsoft/solidus_importer/tree/jardo/configurable-exception-handler