Hypermodel `after_commit` handler is dropping errors instead of raising
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 35/100
Research direction
Start at Hypermodel's after_commit method and inspect the SendPacket.run result, then read InternalPolicy#send_message to understand how promise errors are handled. Done means errors from the after-commit broadcast are raised instead of being dropped, while the server-not-running rescue remains unchanged.
Written by the indexing model from the issue text.
Description
in the after_commit method of hypermodel, the SendPacket operation is run which will return a promise, but it is not checked for an error state.
def self.after_commit(operation, model)
# Calling public_columns_hash once insures all policies are loaded
# before the first broadcast.
@public_columns_hash ||= ActiveRecord::Base.public_columns_hash
Hyperstack::InternalPolicy.regulate_broadcast(model) do |data|
puts "Broadcast aftercommit hook: #{data}" if Hyperstack::Connection.show_diagnostics
if !Hyperstack.on_server? && Hyperstack::Connection.root_path
send_to_server(operation, data, model.__synchromesh_update_time) rescue nil # fails if server no longer running so ignore
else
SendPacket.run(data, operation: operation, updated_at: model.__synchromesh_update_time)
end
end
rescue ActiveRecord::StatementInvalid => e
raise e unless e.message == "Could not find table 'hyperstack_connections'"
end unless RUBY_ENGINE == 'opal'
The line containing SendPacket should be
SendPacket.run(data, operation: operation, updated_at: model.__synchromesh_update_time).tap { |p| raise p.error if p.error }
The easiest way to patch this is by adding this:
module Hyperstack
class InternalPolicy
alias original_send_message send_message
def send_message(*args, &block)
original_send_message(*args, &block).tap { |p| raise p.error if p.is_a?(Promise) && p.error }
end
end
end
This will catch the error with the smallest patch. Note the patch needs to check if its a promise, but the actual fix does not.
- Dominant language
- JavaScript
- Stars
- 538
- Forks
- 41
- PR merge metrics
- No merged PRs in 30d
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from hyperstack-org/hyperstack
-
enhancement needs doc
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
hyperstack-org/hyperstack#359 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
hyperstack-org/hyperstack#267 ·
-
Difficulty 5/5 Over a week Newbie friendliness 25/100
hyperstack-org/hyperstack#457 ·
-
Difficulty 3/5 1-2 days Newbie friendliness 35/100
hyperstack-org/hyperstack#456 ·
-
Difficulty 5/5 Over a week Newbie friendliness 15/100
hyperstack-org/hyperstack#455 · 2 comments ·
All issues in hyperstack-org/hyperstack
Similar issues
-
Bug
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
Automattic/safe-publish#594 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
dream-num/dsh-univer-office#104 ·
-
comp/dashboard invalid P3
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
NousResearch/hermes-agent#121143 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
avniproject/avni-webapp#1811 ·
-
area/auroraboot area/webui bug
Difficulty 2/5 1-3 hours Newbie friendliness 75/100