Unhandled PG::UniqueViolation for topic external_id field

Making an API request to the /posts.json route with the external_id property set to the external_id of an existing topic triggers an error similar to this:

ActiveRecord::RecordNotUnique (PG::UniqueViolation: ERROR: duplicate key value violates unique constraint "index_topics_on_external_id" DETAIL: Key (external_id)=(obCopying text to the system clipboard) already exists.)

When the request is made with the Discourse API gem, the response is just an HTML string:

#<DiscourseApi::Error:"<!DOCTYPE html>\n<html lang=\"en\">\n<head>\n <meta charset=\"utf-8\" />\n ...

If the request is made in some other way, say with HTTParty, the response object looks like this:

response = HTTParty.post(url, body:, headers:)
add_note_to_db(title, response)
# Try getting the `response` object from the response
p response.response

# outputs:
#<Net::HTTPInternalServerError 500 Internal Server Error readbody=true>

There’s not much that can be done with that.

Here are the calls that trigger the issue:

lib/topic_creator.rb:237:in `save_topic'
lib/topic_creator.rb:58:in `create'
lib/post_creator.rb:490:in `create_topic'
lib/post_creator.rb:190:in `block in create'
lib/post_creator.rb:390:in `block in transaction'
lib/post_creator.rb:390:in `transaction'
lib/post_creator.rb:188:in `create'
lib/new_post_manager.rb:318:in `perform_create_post'
lib/new_post_manager.rb:252:in `perform'
app/controllers/posts_controller.rb:210:in `block in create'
lib/distributed_memoizer.rb:16:in `block in memoize'
lib/distributed_mutex.rb:53:in `block in synchronize'
lib/distributed_mutex.rb:49:in `synchronize'
lib/distributed_mutex.rb:49:in `synchronize'
lib/distributed_mutex.rb:34:in `synchronize'
lib/distributed_memoizer.rb:12:in `memoize'
app/controllers/posts_controller.rb:209:in `create'

I think an error needs to be raised in TopicCreator#create, rescued from the transaction block in PostCreator#create, then handled in the posts controller.

The ideal solution from my point of view would be to return an error that indicated which topic was using the external id.

Raise an error in TopicCreator#create:

  def create
    topic = Topic.new(setup_topic_params)

    if topic.external_id.present? && Topic.with_deleted.exists?(external_id: topic.external_id)
      existing_topic = Topic.with_deleted.find_by(external_id: topic.external_id)
      raise DuplicateExternalIdError.new(existing_topic), "External ID must be unique. Existing topic ID: #{existing_topic.id}"

# There's probably an existing error class that could handle this, but sticking this at the bottom of the topic_creator class works for me:

   class DuplicateExternalIdError < StandardError
    attr_reader :topic

    def initialize(topic)
      @topic = topic

Rescue from it in PostCreator#create:

  def create
    if valid?
        transaction do
          create_topic # this is where the error is happening
          # ...
      rescue TopicCreator::DuplicateExternalIdError => e
        @errors.add(:base, "External ID must be unique. Existing topic ID: #{e.topic.id}")

Handle it in the posts controller:

# posts_controller.rb

  def handle_duplicate_external_id_error(exception)
    render json: { error: "External ID must be unique. Existing topic ID: #{exception.topic.id}" }, status: :unprocessable_entity

That’s maybe too specific to my use case to be the right solution. It’s related to the problem with external ids and duplicate topics that’s mentioned here: API topic's external_ID can't be reused after deleting a topic and creating a new one - #2 by simon. Ideally, an application that’s posting to the Discourse API could use response message to undelete and update the topic that was already using the external id.

1 Like

Yes, happy with a PR to improve this error message!

Great! I’ll see what I can do.

It probably doesn’t need to be as specific as what I was proposing yesterday:

rescue TopicCreator::DuplicateExternalIdError => e
   @errors.add(:base, "External ID must be unique. Existing topic ID: #{e.topic.id}")

Just getting a response with details about the what constraint was violated should be good enough.