Refactor homeserver lookup code #12

Merged
bad merged 10 commits from refactor-homeserver-lookup into princess 2020-10-23 08:57:01 +00:00
Showing only changes of commit 1b97351ca0 - Show all commits

View file

@ -129,15 +129,15 @@ class Form extends ElemJS {
this.status(`Looking up homeserver... trying ${address}`)
// Check if we found the actual matrix server
try {
const versionsReq = await fetch(`${address}/_matrix/client/versions`)
if(versionsReq.ok) {
const versions = await versionsReq.json()
if (Array.isArray(versions.versions)) {
return address
}
}
} catch(e) {}
const versionsReq = await fetch(`${address}/_matrix/client/versions`).catch(()=>{})
/* jshint ignore:start */
//JsHint doesn't support optional chaining
// https://github.com/jshint/jshint/issues/3448
if(versionsReq?.ok) {
const versions = await versionsReq.json().catch(()=>{})
if (Array.isArray(versions?.versions)) return address
}
/* jshint ignore:end */
bad marked this conversation as resolved Outdated

Why chain a promise catch here? I don't have a problem with promise chaining, it's just weird considering that we used the try/catch syntax 3 lines up. IMO they should be made consistent?

Why chain a promise catch here? I don't have a problem with promise chaining, it's just weird considering that we used the try/catch syntax 3 lines up. IMO they should be made consistent?
Outdated
Review

It was shorter. I actually wanted to write the code like it's written now but jshint errored on the optional chain. It's fixed now

It was shorter. I actually wanted to write the code like it's written now but jshint errored on the optional chain. It's fixed now
// find the next matrix server in the chain
const root = await fetch(`${address}/.well-known/matrix/client`).then(res => res.json()).catch(e => {