mysociety / alaveteli

Provide a Freedom of Information request system for your jurisdiction
https://alaveteli.org
Other
387 stars 195 forks source link

creating the InfoRequest, OutgoingMessage and InfoRequestEvent should happen in a transaction #1125

Open mhl opened 10 years ago

mhl commented 10 years ago

The following situation happened to me:

  1. I checked out a new branch and tried to make a new test request
  2. I got an error about a prominence column not existing, indicating that I'd forgotten to run bundle exec rake db:migrate after moving to newer code.
  3. From this point on, lots of requests fail with errors like:

    RuntimeError in AdminRequestController#show
    
    internal error, last_event_forming_initial_request gets nil for request 123 outgoing messages count 1 all events: --- []

The problem here seemed to be that the InfoRequest and OutgoingMessage had been created successfully, but creating the InfoRequestEvent had failed. This could be fixed up manually with:

foi_development=# select id from outgoing_messages where info_request_id = 123;
 id 
----
 22

(1 row)
foi_development=# insert into info_request_events (info_request_id, event_type, outgoing_message_id, params_yaml, created_at) values (123, 'sent', 22, E'---\n:outgoing_message_id: 123', now());
INSERT 0 1

However, to avoid people getting into this situation it would be best if the creation of these objects all happened in the same transaction.

henare commented 10 years ago

We recently encountered this situation, where the InfoRequestEvent didn't get created, when a user created a new request.

For some reason, that I've still not been able to determine, Alaveteli thought the address was incorrect and threw an exception before the InfoRequestEvent was created. Here's the details of that issue:

A RuntimeError occurred in request#new:

  invalid email  foi@ipaustralia.gov.au passed to address_from_name_and_email
  lib/mail_handler/backends/mail_backend.rb:358:in `address_from_name_and_email'

Relevant backtrace:

  lib/mail_handler/backends/mail_backend.rb:358:in `address_from_name_and_email'
  app/models/info_request.rb:758:in `recipient_name_and_email'
  app/mailers/outgoing_mailer.rb:23:in `initial_request'
  actionpack (3.1.12) lib/abstract_controller/base.rb:167:in `send_action'
  actionpack (3.1.12) lib/abstract_controller/base.rb:167:in `process_action'
  actionpack (3.1.12) lib/abstract_controller/base.rb:121:in `process'
  actionpack (3.1.12) lib/abstract_controller/rendering.rb:45:in `process'
  actionmailer (3.1.12) lib/action_mailer/old_api.rb:65:in `process'
  actionmailer (3.1.12) lib/action_mailer/base.rb:474:in `process'
  actionmailer (3.1.12) lib/action_mailer/base.rb:469:in `initialize'
  actionmailer (3.1.12) lib/action_mailer/base.rb:456:in `new'
  actionmailer (3.1.12) lib/action_mailer/base.rb:456:in `method_missing'
  app/models/outgoing_message.rb:155:in `send_message'
  app/controllers/request_controller.rb:352:in `new'

Similar to @mhl above, I manually created the InfoRequestEvent but used the Rails console instead. This allowed me to see the request page and resend the message using the admin backend.

crowbot commented 10 years ago

I think the problem with the address may be a leading space. If so, the immediate action here may be to figure out why it wasn't stripped.

henare commented 10 years ago

I couldn't see a problem in the admin UI and we'd sent requests to this address before.

henare commented 10 years ago

Also, the first thing I did was fire up a Rails console and manually run that function and it worked fine.

crowbot commented 10 years ago

I'd have a quick check in the db to see if there's a leading space (from the error message you pasted, it looks like there is). Previous versions of Alaveteli would have considered an email address with a leading space as OK, so it might have been fine previously if you were running an Alaveteli version before 0.12 at the time.

henare commented 10 years ago

Ah that could have been it. It's all working now so I can't go back and check [easily].

garethrees commented 6 years ago

Just noting some documentation for manually resending messages that get this error https://github.com/mysociety/alaveteli/wiki/Resending-Failed-Outgoing-Messages