Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mapping conflict exception not bubbling up to root cause #12839

Closed
clintongormley opened this issue Aug 12, 2015 · 3 comments
Closed

Mapping conflict exception not bubbling up to root cause #12839

clintongormley opened this issue Aug 12, 2015 · 3 comments
Assignees
@nik9000
Labels
>bug :Core/Infra/Core Core issues without another label good first issue low hanging fruit v2.1.0

Comments

@clintongormley
Copy link

A mapping conflict such as:

PUT my_index
{
  "mappings": {
    "type_one": {
      "properties": {
        "text": {
          "type": "string",
          "analyzer": "standard",
          "search_analyzer": "whitespace"
        }
      }
    },
    "type_two": {
      "properties": {
        "text": {
          "type": "string"
        }
      }
    }
  }
}

throws an exception like:

    {
       "error": {
          "root_cause": [
             {
                "type": "mapper_parsing_exception",
                "reason": "mapping [type_two]",
                "stack_trace": "MapperParsingException[mapping [type_two]]; ... 6 more\n"
             }
          ],
          "type": "mapper_parsing_exception",
          "reason": "mapping [type_two]",
          "caused_by": {
             "type": "illegal_argument_exception",
             "reason": "Mapper for [text] conflicts with existing mapping in other types:\n[mapper [text] has different analyzer, mapper [text] is used by multiple types. Set update_all_types to true to update [search_analyzer] across all types.]",
             "stack_trace": "java.lang....\n"
          },
          "stack_trace": "MapperParsingException[mapping [type_two]]; nested: IllegalArgumentException[Mapper for [text] conflicts with existing mapping in other types:\n[mapper [text] has different analyzer, mapper [text] is used by multiple types. Set update_all_types to true to update [search_analyzer] across all types.]];...\n"
       },
       "status": 400
    }

The error.root_cause.reason should contain the text from error.type.caused_by.reason
xuzha pushed a commit to xuzha/elasticsearch that referenced this issue Aug 13, 2015
Bubbling up the mapping exception msg.
fe531c0 
closes elastic#12839
@xuzha xuzha mentioned this issue Aug 13, 2015
Closed
@xuzha
Copy link
Contributor

xuzha commented Aug 14, 2015

I'm not sure if this is a bug.
The root causes here block looks like correct. Inside of it, there could be a caused_by key-value pair like this:

          "root_cause": [
             {
                "type": "mapper_parsing_exception",
                "reason": "mapping [type_two]",
                "stack_trace": "MapperParsingException[mapping [type_two]]; ... 6 more\n"
                "caused_by": {.......} // skipped
             }

We skip it on purpose, here

@rjernst
Copy link
Member

rjernst commented Aug 14, 2015

I don't think it is a bug, but a choice that was made. The root_cause stops at a subclass of ElasticsearchException, I believe. Perhaps @s1monw can elaborate on why we don't go down to the original root cause (the IAE here)?

@jpountz
Copy link
Contributor

jpountz commented Aug 14, 2015

+1 on improving exception rendering to try to better expose the actual root cause of the issue

@clintongormley clintongormley added the good first issue low hanging fruit label Oct 6, 2015
@nik9000 nik9000 self-assigned this Oct 6, 2015
@nik9000 nik9000 added v2.1.0 and removed v2.0.0 help wanted adoptme labels Oct 6, 2015
@nik9000 nik9000 mentioned this issue Oct 6, 2015
Merged
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Oct 19, 2015
@nik9000
Make root_cause of field conflicts more obvious
1cb3068 
Does so by improving the error message passed to MapperParsingException.

The error messages for mapping conflicts now look like:
```
{
  "error" : {
    "root_cause" : [ {
      "type" : "mapper_parsing_exception",
      "reason" : "Failed to parse mapping [type_one]: Mapper for [text] conflicts with existing mapping in other types:\n[mapper [text] has different [analyzer], mapper [text] is used by multiple types. Set update_all_types to true to update [search_analyzer] across all types., mapper [text] is used by multiple types. Set update_all_types to true to update [search_quote_analyzer] across all types.]"
    } ],
    "type" : "mapper_parsing_exception",
    "reason" : "Failed to parse mapping [type_one]: Mapper for [text] conflicts with existing mapping in other types:\n[mapper [text] has different [analyzer], mapper [text] is used by multiple types. Set update_all_types to true to update [search_analyzer] across all types., mapper [text] is used by multiple types. Set update_all_types to true to update [search_quote_analyzer] across all types.]",
    "caused_by" : {
      "type" : "illegal_argument_exception",
      "reason" : "Mapper for [text] conflicts with existing mapping in other types:\n[mapper [text] has different [analyzer], mapper [text] is used by multiple types. Set update_all_types to true to update [search_analyzer] across all types., mapper [text] is used by multiple types. Set update_all_types to true to update [search_quote_analyzer] across all types.]"
    }
  },
  "status" : 400
}
```

Closes elastic#12839

Change implementation

Rather than make a new exception this improves the error message of the old
exception.
nik9000 added a commit that referenced this issue Oct 19, 2015
@nik9000
Make root_cause of field conflicts more obvious
2a498f1 
Does so by improving the error message passed to MapperParsingException.

The error messages for mapping conflicts now look like:
```
{
  "error" : {
    "root_cause" : [ {
      "type" : "mapper_parsing_exception",
      "reason" : "Failed to parse mapping [type_one]: Mapper for [text] conflicts with existing mapping in other types:\n[mapper [text] has different [analyzer], mapper [text] is used by multiple types. Set update_all_types to true to update [search_analyzer] across all types., mapper [text] is used by multiple types. Set update_all_types to true to update [search_quote_analyzer] across all types.]"
    } ],
    "type" : "mapper_parsing_exception",
    "reason" : "Failed to parse mapping [type_one]: Mapper for [text] conflicts with existing mapping in other types:\n[mapper [text] has different [analyzer], mapper [text] is used by multiple types. Set update_all_types to true to update [search_analyzer] across all types., mapper [text] is used by multiple types. Set update_all_types to true to update [search_quote_analyzer] across all types.]",
    "caused_by" : {
      "type" : "illegal_argument_exception",
      "reason" : "Mapper for [text] conflicts with existing mapping in other types:\n[mapper [text] has different [analyzer], mapper [text] is used by multiple types. Set update_all_types to true to update [search_analyzer] across all types., mapper [text] is used by multiple types. Set update_all_types to true to update [search_quote_analyzer] across all types.]"
    }
  },
  "status" : 400
}
```

Closes #12839

Change implementation

Rather than make a new exception this improves the error message of the old
exception.
nik9000 added a commit that referenced this issue Oct 19, 2015
@nik9000
Make root_cause of field conflicts more obvious
74d87bf 
Does so by improving the error message passed to MapperParsingException.

The error messages for mapping conflicts now look like:
```
{
  "error" : {
    "root_cause" : [ {
      "type" : "mapper_parsing_exception",
      "reason" : "Failed to parse mapping [type_one]: Mapper for [text] conflicts with existing mapping in other types:\n[mapper [text] has different [analyzer], mapper [text] is used by multiple types. Set update_all_types to true to update [search_analyzer] across all types., mapper [text] is used by multiple types. Set update_all_types to true to update [search_quote_analyzer] across all types.]"
    } ],
    "type" : "mapper_parsing_exception",
    "reason" : "Failed to parse mapping [type_one]: Mapper for [text] conflicts with existing mapping in other types:\n[mapper [text] has different [analyzer], mapper [text] is used by multiple types. Set update_all_types to true to update [search_analyzer] across all types., mapper [text] is used by multiple types. Set update_all_types to true to update [search_quote_analyzer] across all types.]",
    "caused_by" : {
      "type" : "illegal_argument_exception",
      "reason" : "Mapper for [text] conflicts with existing mapping in other types:\n[mapper [text] has different [analyzer], mapper [text] is used by multiple types. Set update_all_types to true to update [search_analyzer] across all types., mapper [text] is used by multiple types. Set update_all_types to true to update [search_quote_analyzer] across all types.]"
    }
  },
  "status" : 400
}
```

Closes #12839

Change implementation

Rather than make a new exception this improves the error message of the old
exception.
@lcawl lcawl added :Core/Infra/Core Core issues without another label and removed :Exceptions labels Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nik9000 nik9000

Labels
>bug :Core/Infra/Core Core issues without another label good first issue low hanging fruit v2.1.0
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@clintongormley @nik9000 @rjernst @jpountz @xuzha @lcawl