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

Fix reporting errors from Oban #70

Merged
merged 4 commits into from
Aug 28, 2024
Merged

Conversation

odarriba
Copy link
Contributor

@odarriba odarriba commented Aug 27, 2024

There was an error when an Oban worker returned an {:error, reason} tuple because Oban does not return an stacktrace (makes sense, as it is not an exception by itself).

Our code was not prepared for that use case. I modified it to handle it and circumvent the needed for source file and source function information.

After this change, empty stack traces and source information are not shown on the dashboard to avoid confusion.

As an extra feature I have also added the reporting of the state of the job (which is mostly ever failure for us).

How to test

The way to reproduce it is to have an Oban worker which returns a tuple {:error, any()} or just :error.

That use case generates an exception in the latest release and works as expected on this branch.


Closes #63

@odarriba odarriba added the bug Something isn't working label Aug 27, 2024
@odarriba odarriba requested a review from crbelaus August 27, 2024 18:48
@odarriba odarriba self-assigned this Aug 27, 2024
@jaimeiniesta
Copy link

Hey, that works great! I have tested this branch and now the error is tracked, and I can see the job details in the error context, as expected.

Thanks! 🚀

Comment on lines 125 to 126
defp error_has_source_info?(%Error{source_function: "-", source_line: "-"}), do: false
defp error_has_source_info?(%Error{}), do: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can move this function to ErrorTracker.Error.has_source_info? so we can reuse it both in show and dashboard templates. We are currently using it in the show template but doing the check manually in the dashboard template.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in e658416

Copy link
Contributor

@crbelaus crbelaus left a comment

Choose a reason for hiding this comment

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

I've tested it before and after the change and it works perfectly.

I am wondering if we should add Oban as an optional dependency and create a test for this. Mainly to ensure that newer Oban versions don't break our assumptions.

Anyway this can be done later so feel free to merge it as-is.

There was an error when an Oban worker returned an `{:error, reason}`
tuple because Oban does not return an stacktrace (makes sense, as
it is not an exception by itself).

Our code was not prepared for that use case. I modified it to handle
it and circunvent the needed for source file and source function
information.

As an extra feature I have also added the reporting of the `state`
of the job (which is mostly ever `failure` for us).
@odarriba odarriba force-pushed the fix-report-errors-from-oban branch from 420c3dc to 288bc2d Compare August 28, 2024 18:27
@odarriba odarriba force-pushed the fix-report-errors-from-oban branch from 288bc2d to e658416 Compare August 28, 2024 18:31
@odarriba odarriba merged commit 3886369 into main Aug 28, 2024
3 checks passed
@odarriba odarriba deleted the fix-report-errors-from-oban branch August 28, 2024 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handler ErrorTracker.Integrations.Oban has failed and has been detached
3 participants