Skip to content
This repository has been archived by the owner on Oct 24, 2024. It is now read-only.

Fixing arity mismatch for ImportUrlJob#send_error #365

Merged
merged 2 commits into from
Apr 10, 2023

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Apr 10, 2023

Fixing arity mismatch for ImportUrlJob#send_error

7b21584

In v2.9.6 of Hyrax we saw the following error:

File Import Error
Error: wrong number of arguments (given 2, expected 1)

It masks other errors, because the message sending for errors is itself
raising an error.

With this change, we ignore all extra parameters and pass the first
parameter.

Related to:

Resolved in:

Adding logging for muted exception

811b738

Prior to this commit, we were taking an exception and passing just the
message to the user. We abandoned the useful backtrace and context.

With this commit, we add error logging of the back trace.

An outstanding question is "What would be the consequence of re-raising
the exception?" Would this help us gain better insight into what's
going on? It seems like we probably should do that.

In v2.9.6 of Hyrax we saw the following error:

> File Import Error
>   Error: wrong number of arguments (given 2, expected 1)

It masks other errors, because the message sending for errors is itself
raising an error.

With this change, we ignore all extra parameters and pass the first
parameter.

Related to:

- samvera/hyrax#2821

Resolved in:

- samvera/hyrax#4243
- samvera/hyrax@bfd316e
Prior to this commit, we were taking an exception and passing just the
message to the user.  We abandoned the useful backtrace and context.

With this commit, we add error logging of the back trace.

An outstanding question is "What would be the consequence of re-raising
the exception?"  Would this help us gain better insight into what's
going on?  It seems like we probably should do that.
@jeremyf jeremyf merged commit d8642c7 into main Apr 10, 2023
@jeremyf jeremyf deleted the patching-issue-with-import-url-job branch April 10, 2023 17:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants