Skip to content

Handle doc lookup where id not found, and remove problematic hash-to-query addon #592

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

Merged
merged 2 commits into from
Mar 28, 2019
Merged

Handle doc lookup where id not found, and remove problematic hash-to-query addon #592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a38fa1
Handle doc lookup where id not found, and remove problematic hash-to-…
toddjordan Feb 7, 2019
2f2b117
Prevent double fetch when a call find with namespace type fails
toddjordan Feb 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions app/adapters/application.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,12 @@ export default JSONAPIAdapter.extend({
modelNameToUse = 'namespace';
}

let encodedRevId = encodeURIComponent(revId);
url = `json-docs/${projectName}/${version}/${pluralize(modelNameToUse)}/${encodedRevId}`;
if (typeof revId != 'undefined') {
let encodedRevId = encodeURIComponent(revId);
url = `json-docs/${projectName}/${version}/${pluralize(modelNameToUse)}/${encodedRevId}`;
} else {
throw new Error('Documentation item not found');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this just trading a "cannot access blah of undefined" error with a "Documentation item not found" error?

My experience with the issue of thrown errors causing potential memory leaks is that any kind of error will cause it 🤔 Would we be able to console.log() this message and then just return null or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we catch the error in the calling route model function, and then handle it. My assumption was that it was the uncaught error that was causing it, which was happening because we were letting the error happen deep in the nodejs fetch code as opposed to here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that sounds legit to me 🎉 😂 I wasn't actually sure if you handled the error anywhere or if it ended up being "unhandled" from Ember's perspective

}
} else if (modelName === 'missing') {
let version = this.get('projectService.version');
let revId = this.get('metaStore').getRevId(projectName, version, modelName, id);
Expand Down
18 changes: 14 additions & 4 deletions app/routes/project-version/classes/class.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,20 @@ export default Route.extend(ScrollTracker, {

find(typeName, param) {
return this.store.find(typeName, param).catch((e1) => {
Logger.warn(e1, 'fetching by class or module failed, retrying as namespace');
return this.store.find('namespace', param).catch((e2) => {
Logger.error(e2);
return resolve({ isError: true });
if (typeName != 'namespace') {
Logger.warn(e1, 'fetching by class or module failed, retrying as namespace');
return this.store.find('namespace', param).catch((e2) => {
Logger.error(e2);
return resolve({
isError: true,
status: 404
});
});
}
Logger.error(e1);
return resolve({
isError: true,
status: 404
});
});
},
Expand Down
33 changes: 0 additions & 33 deletions lib/hash-to-query/index.js
View file
Open in desktop

This file was deleted.

6 changes: 0 additions & 6 deletions lib/hash-to-query/package.json
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,5 @@
"npm": "5"
},
"cacheDirectories": ["bower_components", "node_modules"],
"ember-addon": {
"paths": ["lib/hash-to-query"]
},
"fastbootDependencies": ["algoliasearch"]
}