Skip to content

Fix NoMethodError in JobWebhook when error is nil#222

Merged
sebastianMindee merged 2 commits intomindee:mainfrom
HarisDelalic:fix/job-webhook-nil-error
Feb 5, 2026
Merged

Fix NoMethodError in JobWebhook when error is nil#222
sebastianMindee merged 2 commits intomindee:mainfrom
HarisDelalic:fix/job-webhook-nil-error

Conversation

@HarisDelalic
Copy link
Contributor

@HarisDelalic HarisDelalic commented Feb 5, 2026

Summary

Fix NoMethodError in JobWebhook#initialize when the error key is present but value is nil.

Problem

When the API returns a response like:

{"id": "12345678-1234-1234-1234-123456789012", "status": "Processing", "error": null}

The JobWebhook initializer would call ErrorResponse.new(nil), causing:
NoMethodError: undefined method '[]' for nil

Solution

Updated JobWebhook#initialize to match the existing pattern in Job#initialize:
unless server_response['error'].nil? || server_response['error'].empty?
  @error = ErrorResponse.new(server_response['error'])
end

Test plan

- Added unit tests for JobWebhook covering all edge cases

<img width="1460" height="352" alt="PR_description_img" src="https://github.com/user-attachments/assets/64591692-8d35-4d88-9b43-ef0596d717de" />

Copy link
Collaborator

@sebastianMindee sebastianMindee left a comment

Choose a reason for hiding this comment

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

First off, thank you very much for taking the time to do this. The vast majority of people don't care enough to help, so this is like a breath of fresh air.

Secondly, there is a couple of issue with the PR I'd like you to fix if possible:

  • The error checks don't need to account for {}. Both in the test and the constructor.
  • The error type is correct in the docstring but invalid in steep (my bad), can you update it? should be attr_reader error: ErrorResponse? instead of attr_reader error: ErrorResponse in sig/mindee/parsing/v2/job_webhook.rbs .
  • The linting checks are failing for me. Please run bundle exec rubocop -A before committing.

Once done, I can release later today or tomorrow at the latest if all goes well.

Thank you!

server_response = {
'id' => '12345678-1234-1234-1234-123456789012',
'status' => 'Processing',
'error' => {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should never happen. And I think it would be important for this to break if an unexpected error was loaded.

Comment on lines 26 to 28
unless server_response['error'].nil? || server_response['error'].empty?
@error = ErrorResponse.new(server_response['error'])
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
unless server_response['error'].nil? || server_response['error'].empty?
@error = ErrorResponse.new(server_response['error'])
end
unless server_response['error'].nil?
@error = ErrorResponse.new(server_response['error'])
end

Empty error shouldn't be possible. Only valid objects or null.

Copy link
Collaborator

@sebastianMindee sebastianMindee left a comment

Choose a reason for hiding this comment

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

Amazing thanks 🙏

@sebastianMindee sebastianMindee merged commit 4b38722 into mindee:main Feb 5, 2026
41 of 50 checks passed
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.

2 participants