Excerpt gets too long under certain circumstances: "ActiveRecord::ValueTooLong"

[niko]

Under certain circumstances the excerpt gets longer than 1000 characters and the server throws a 500 and logs ActiveRecord::ValueTooLong (PG::StringDataRightTruncation: ERROR: value too long for type character varying(1000) : UPDATE "topics" SET "excerpt" = '<details><summary>Ich habe meine Playlisten gefüll.

We stumbled over this when we where trying to save a topic consisting only of detail/summary content. It can be reproduced by trying to save this as a new topic:

[details="Some question?"]
Some answer.
[/details]

[details="Some question?"]
Some answer.
[/details]

[details="Some question?"]
Some answer.
[/details]

[details="Some question?"]
Some answer.
[/details]

[details="Some question?"]
Some answer.
[/details]

[details="Some question?"]
Some answer.
[/details]

[details="Some question?"]
Some answer.
[/details]

[details="Some question?"]
Some answer.
[/details]

[details="Some question?"]
Some answer.
[/details]

[details="Some question?"]
Some answer.
[/details]

[details="Some question?"]
Some answer.
[/details]

[details="Some question?"]
Some answer.
[/details]

[details="Some question?"]
Some answer.
[/details]

[details="Some question?"]
Some answer.
[/details]

[details="Some question?"]
Some answer.
[/details]

[details="Some question?"]
Some answer.
[/details]

… and works fine when removing one of the details blocks. As a work around we now added a ~ 200 char paragraph before the Q/A section.

Backtrace

/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-mini-profiler-0.10.7/lib/patches/db/pg.rb:93:in `exec'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-mini-profiler-0.10.7/lib/patches/db/pg.rb:93:in `async_exec'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/postgresql_adapter.rb:614:in `block (2 levels) in exec_no_cache'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-5.1.4/lib/active_support/dependencies/interlock.rb:46:in `block in permit_concurrent_loads'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-5.1.4/lib/active_support/concurrency/share_lock.rb:185:in `yield_shares'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-5.1.4/lib/active_support/dependencies/interlock.rb:45:in `permit_concurrent_loads'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/postgresql_adapter.rb:613:in `block in exec_no_cache'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/abstract_adapter.rb:612:in `block (2 levels) in log'
/usr/local/lib/ruby/2.3.0/monitor.rb:214:in `mon_synchronize'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/abstract_adapter.rb:611:in `block in log'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-5.1.4/lib/active_support/notifications/instrumenter.rb:21:in `instrument'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/abstract_adapter.rb:603:in `log'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/postgresql_adapter.rb:612:in `exec_no_cache'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/postgresql_adapter.rb:599:in `execute_and_clear'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:92:in `exec_delete'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/abstract/database_statements.rb:140:in `update'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/abstract/query_cache.rb:17:in `update'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/relation.rb:380:in `update_all'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/persistence.rb:333:in `update_columns'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/persistence.rb:306:in `update_column'
/var/www/discourse/lib/post_revisor.rb:436:in `update_topic_excerpt'
/var/www/discourse/lib/post_revisor.rb:430:in `revise_topic'
/var/www/discourse/lib/post_revisor.rb:162:in `block in revise!'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/abstract/database_statements.rb:235:in `block in transaction'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/abstract/transaction.rb:194:in `block in within_new_transaction'
/usr/local/lib/ruby/2.3.0/monitor.rb:214:in `mon_synchronize'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/abstract/transaction.rb:191:in `within_new_transaction'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/abstract/database_statements.rb:235:in `transaction'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/transactions.rb:210:in `transaction'
/var/www/discourse/lib/post_revisor.rb:150:in `revise!'
/var/www/discourse/app/controllers/posts_controller.rb:193:in `update'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_controller/metal/basic_implicit_render.rb:4:in `send_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/abstract_controller/base.rb:186:in `process_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_controller/metal/rendering.rb:30:in `process_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/abstract_controller/callbacks.rb:20:in `block in process_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-5.1.4/lib/active_support/callbacks.rb:131:in `run_callbacks'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/abstract_controller/callbacks.rb:19:in `process_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_controller/metal/rescue.rb:20:in `process_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_controller/metal/instrumentation.rb:32:in `block in process_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-5.1.4/lib/active_support/notifications.rb:166:in `block in instrument'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-5.1.4/lib/active_support/notifications/instrumenter.rb:21:in `instrument'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-5.1.4/lib/active_support/notifications.rb:166:in `instrument'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_controller/metal/instrumentation.rb:30:in `process_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_controller/metal/params_wrapper.rb:252:in `process_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-5.1.4/lib/active_record/railties/controller_runtime.rb:22:in `process_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/abstract_controller/base.rb:124:in `process'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionview-5.1.4/lib/action_view/rendering.rb:30:in `process'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-mini-profiler-0.10.7/lib/mini_profiler/profiling_methods.rb:102:in `block in profile_method'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_controller/metal.rb:189:in `dispatch'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_controller/metal.rb:253:in `dispatch'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_dispatch/routing/route_set.rb:49:in `dispatch'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_dispatch/routing/route_set.rb:31:in `serve'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_dispatch/journey/router.rb:50:in `block in serve'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_dispatch/journey/router.rb:33:in `each'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_dispatch/journey/router.rb:33:in `serve'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_dispatch/routing/route_set.rb:834:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-protection-2.0.0/lib/rack/protection/frame_options.rb:31:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:189:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:167:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:189:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:167:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:189:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:167:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:189:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:167:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:189:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:167:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:189:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:167:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/builder.rb:63:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-2.0.3/lib/rack/conditional_get.rb:38:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-2.0.3/lib/rack/head.rb:12:in `call'
/var/www/discourse/lib/middleware/anonymous_cache.rb:149:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-2.0.3/lib/rack/session/abstract/id.rb:232:in `context'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-2.0.3/lib/rack/session/abstract/id.rb:226:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_dispatch/middleware/cookies.rb:613:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_dispatch/middleware/callbacks.rb:26:in `block in call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-5.1.4/lib/active_support/callbacks.rb:97:in `run_callbacks'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_dispatch/middleware/callbacks.rb:24:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_dispatch/middleware/debug_exceptions.rb:59:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_dispatch/middleware/show_exceptions.rb:31:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/logster-1.2.9/lib/logster/middleware/reporter.rb:31:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/railties-5.1.4/lib/rails/rack/logger.rb:36:in `call_app'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/railties-5.1.4/lib/rails/rack/logger.rb:26:in `call'
/var/www/discourse/config/initializers/100-quiet_logger.rb:16:in `call'
/var/www/discourse/config/initializers/100-silence_logger.rb:29:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_dispatch/middleware/remote_ip.rb:79:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_dispatch/middleware/request_id.rb:25:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-2.0.3/lib/rack/method_override.rb:22:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-2.0.3/lib/rack/runtime.rb:22:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-5.1.4/lib/action_dispatch/middleware/executor.rb:12:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-2.0.3/lib/rack/sendfile.rb:111:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-mini-profiler-0.10.7/lib/mini_profiler/profiler.rb:282:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/message_bus-2.1.2/lib/message_bus/rack/middleware.rb:63:in `call'
/var/www/discourse/lib/middleware/request_tracker.rb:138:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/railties-5.1.4/lib/rails/engine.rb:522:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/railties-5.1.4/lib/rails/railtie.rb:185:in `public_send'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/railties-5.1.4/lib/rails/railtie.rb:185:in `method_missing'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-2.0.3/lib/rack/urlmap.rb:68:in `block in call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-2.0.3/lib/rack/urlmap.rb:53:in `each'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-2.0.3/lib/rack/urlmap.rb:53:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/unicorn-5.3.1/lib/unicorn/http_server.rb:606:in `process_client'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/unicorn-5.3.1/lib/unicorn/http_server.rb:702:in `worker_loop'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/unicorn-5.3.1/lib/unicorn/http_server.rb:549:in `spawn_missing_workers'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/unicorn-5.3.1/lib/unicorn/http_server.rb:142:in `start'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/unicorn-5.3.1/bin/unicorn:126:in `<top (required)>'
/var/www/discourse/vendor/bundle/ruby/2.3.0/bin/unicorn:22:in `load'
/var/www/discourse/vendor/bundle/ruby/2.3.0/bin/unicorn:22:in `<main>'

[codinghorror (Jeff Atwood)]

So this only happens with tons of repeated [details] elements, can you repro with one giant [details] element or does it take many?

[niko]

In our case it was 9. I could reduce it to 5 but no further no matter what size each [details] body is:

[details="1-10"]
* **one** [discourse](http://discourse.org)
* **two** [discourse](http://discourse.org)
* **three** [discourse](http://discourse.org)
* **four** [discourse](http://discourse.org)
* **five** [discourse](http://discourse.org)
* **six** [discourse](http://discourse.org)
* **seven** [discourse](http://discourse.org)
* **eight** [discourse](http://discourse.org)
* **nine** [discourse](http://discourse.org)
* **ten** [discourse](http://discourse.org)
[/details]

[details="1-10"]
* **one** [discourse](http://discourse.org)
* **two** [discourse](http://discourse.org)
* **three** [discourse](http://discourse.org)
* **four** [discourse](http://discourse.org)
* **five** [discourse](http://discourse.org)
* **six** [discourse](http://discourse.org)
* **seven** [discourse](http://discourse.org)
* **eight** [discourse](http://discourse.org)
* **nine** [discourse](http://discourse.org)
* **ten** [discourse](http://discourse.org)
[/details]

[details="1-10"]
* **one** [discourse](http://discourse.org)
* **two** [discourse](http://discourse.org)
* **three** [discourse](http://discourse.org)
* **four** [discourse](http://discourse.org)
* **five** [discourse](http://discourse.org)
* **six** [discourse](http://discourse.org)
* **seven** [discourse](http://discourse.org)
* **eight** [discourse](http://discourse.org)
* **nine** [discourse](http://discourse.org)
* **ten** [discourse](http://discourse.org)
[/details]

[details="1-10"]
* **one** [discourse](http://discourse.org)
* **two** [discourse](http://discourse.org)
* **three** [discourse](http://discourse.org)
* **four** [discourse](http://discourse.org)
* **five** [discourse](http://discourse.org)
* **six** [discourse](http://discourse.org)
* **seven** [discourse](http://discourse.org)
* **eight** [discourse](http://discourse.org)
* **nine** [discourse](http://discourse.org)
* **ten** [discourse](http://discourse.org)
[/details]

[details="1-10"]
* **one** [discourse](http://discourse.org)
* **two** [discourse](http://discourse.org)
* **three** [discourse](http://discourse.org)
* **four** [discourse](http://discourse.org)
* **five** [discourse](http://discourse.org)
* **six** [discourse](http://discourse.org)
* **seven** [discourse](http://discourse.org)
* **eight** [discourse](http://discourse.org)
* **nine** [discourse](http://discourse.org)
* **ten** [discourse](http://discourse.org)
[/details]

It feels like the excerpt parser limits the number of characters based on a plain text representation, disregarding any markup but I couldn’t find the corresponding code. There’s a lot going on there… character counting with conditionals on content context, html escaping etc. … so somehow all this hits a pathologic case sometimes.

[niko]

I guess it basically boils down to ExcerptParser not stripping <details> tags (due to this?) but ignoring these while calculating the length:

bundle exec pry
[5] pry(main)> require 'nokogiri'
[6] pry(main)> require_relative 'lib/excerpt_parser.rb'

[27] pry(main)> s = '<h1>foo</h1><p>bar</p>'
=> "<h1>foo</h1><p>bar</p>"
[33] pry(main)> ExcerptParser.get_excerpt(s*1000, 220, strip_links: true).size
=> 229

[34] pry(main)> s = '<details><summary>foo</summary><p>bar</p></details>'
=> "<details><summary>foo</summary><p>bar</p></details>"
[35] pry(main)> ExcerptParser.get_excerpt(s*1000, 220, strip_links: true).size
=> 44000

(And then of course PostGres - doing the right thing - throwing an error when a string is longer than the column.)

But I actually don’t know if that’s even in the code path executed when saving a topic.

[gerhard (Gerhard Schlager)]

Yeah, I wonder if we should exclude the details tag from the excerpt or at least include only the summary.

[wesochuck (Wes Osborn)]

I’m getting the same thing on Stable 1.9 on a post with lots of [details] elements in it.

[riking (Kane York)]

@gerhard

or at least include only the summary.

Include, and count the summary, of course.

It’ll still fail with repeated extremely short summaries, so you could set a floor of a details block always counting for at least 40 characters or so (because they take vertical space, so you don’t want more than about 2 in an excerpt).

[gerhard (Gerhard Schlager)]

I fixed the length calculation when the excerpt contains details blocks:
FIX: Calculation of text length for <details> in excerpt was wrong · discourse/discourse@d243b82 · GitHub

There’s still a chance that lots of small details blocks or a combination of other HTML tags could bring you over the 1000 character limit, but it’s a lot less likely. Fixing this completely would require removal of the column’s length limit.