Added 'did you mean' suggestions middleware #372
Conversation
Cawllec
commented
Sep 25, 2017
Cawllec
commented
- Adds middleware to capture 'did you mean' suggestions and append them to meta-data in ruby version > 2.3.0
- Uses string matching to not break earlier versions
- Includes tests to ensure early ruby version run correctly
| class SuggestionData | ||
|
|
||
| CAPTURE_REGEX = /Did you mean\?([\s\S]+)$/ | ||
| DELIMITTER = "\n" |
There was a problem hiding this comment.
I think the extra 'T' in "delimiter" is a typo here
| } | ||
| end | ||
|
|
||
| report.add_tab(:"did you mean", matches) if matches.size > 0 |
There was a problem hiding this comment.
Is there a case for this to happen multiple times in a single exception? Otherwise I think we can better name the metadata field something like suggestion so that it is filterable by users rather than using an index.
There was a problem hiding this comment.
I've changed the tab name to suggestions, is there a better method of attaching the suggestions than indexed in a hash? It seems like the library blocks directly attaching an array as a tab.
There was a problem hiding this comment.
Naming the tab "did you mean" is fine, the matches could instead be placed in a hash:
{suggestions: matches}Though since there's usually one, we should instead unwrap it:
if matches.size == 1
report.add_tab(:"did you mean", {suggestion: matches.first}
elsif matches.size > 1
# add suggestions array, as above
end
| end | ||
|
|
||
| if matches.size == 1 | ||
| report.add_tab(:"did you mean", {:suggestion => matches.first}) |
There was a problem hiding this comment.
Commonly in other notifiers we use an error tab i believe to generically have information about the error? Having a tab with one entry in it doesnt seem like something we would want to do long term
There was a problem hiding this comment.
That's a good idea. We could use "Error" as the tab name, in case there is other error-specific items to add later.
There was a problem hiding this comment.
As an idea I just had - its where the bugsnag_meta_data attached to the error could go?
There was a problem hiding this comment.
Would that risk making the 'custom' tab redundant? It might be useful creating an 'error' tab and renaming the 'custom' tab to separate error generated vs user generated
There was a problem hiding this comment.
I dont understand the concern? The custom tab only shows up if someone puts something there. I was just saying if you attach metadata to the error and there was an error tab I would expect it to show up there.
Not a big deal though.
There was a problem hiding this comment.
@Cawllec The custom tab is where metadata is collated if it does not conform to the expected format (e.g., not nested so that there is a tab name and a key name, like if you use report.add_tab(:error, "foo")). Its a bit out of scope of this changeset, but I think I see what you mean. Let's use :error as the tab name here though, and then we could separately use it as the tab for bugsnag_meta_data in a later feature.
