Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failure backend #36

Merged
merged 6 commits into from
Jul 24, 2017
Merged

Failure backend #36

merged 6 commits into from
Jul 24, 2017

Conversation

ono
Copy link
Contributor

@ono ono commented Jul 21, 2017

The change allows you to implement/configure custom job failure backends.

config :task_bunny, failure_backend: [
  TaskBunny.FailureBackend.Logger,
  YourApp.CustomFailureBackend
]

This will allow you to send the error report to your choice of services such as Rollbar, Airbrake, Bugsnag, Raygun etc... with the richer information.

We are also planning to release Rollbar backend as a separate hex library sometime in the next week.

@@ -0,0 +1,55 @@
defmodule TaskBunny.FailureBackend do
@moduledoc """
A behaviour module to implment the your own failure backend.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: implement

Copy link
Contributor

@IanLuites IanLuites left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! 👍 Looks really good, can't wait to use it and debug those errors!

I do want to mention that the following files miss @spec for the public functions:

  • lib/task_bunny/failure_backend/logger.ex
  • lib/task_bunny/job_error.ex

@doc """
Returns the list of failure backends.

It returns [TaskBunny.FailureBackend.Logger] by default.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that if you put TaskBunny.FailureBackend.Logger between backticks that ExDoc will pick up on the module and make it linkable.

Ref: Auto-Linking

Config.failure_backend()
|> Enum.each(&(&1.report_job_error(job_error)))

:ok
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really necessary, the result of Enum.each/2 is already :ok. (Isn't that bad either.)

#{common_message error}
"""

do_report(message, error.reject)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More an option suggestion, I know your preference, so I'm just putting it out there, because it will align the triple-quotes nicely.

If you like it you can also write:

"""
TaskBunny - #{error.job} failed for an invalid return value.

Return value:
#{my_inspect error.return_value}

#{common_message error}
"""
|> do_report(error.reject)

(You know how I love aligning and pipes 😛)

}

defstruct [
job: nil,
Copy link
Contributor

@IanLuites IanLuites Jul 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value of :job is defined as only atom, so it might be nicer to write :job, here in stead of job: nil. (Because that would be a default that is technically not allowed by the spec.)

If you go for :job then I would also add @enforce_keys [:job] so it also does not default to nil.

The same goes for :pid and turning it into @enforce_keys [:job, :pid].

@ono ono force-pushed the feature/failure-backend branch from c0908fe to 93100c6 Compare July 24, 2017 09:10
@ono ono force-pushed the feature/failure-backend branch from 93100c6 to d3dc0c5 Compare July 24, 2017 09:13
@ono ono merged commit de27174 into master Jul 24, 2017
@ono ono deleted the feature/failure-backend branch July 24, 2017 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants