cadence/Carbon
3
9
Fork 1
Code Issues 14 Pull requests 1 Releases Wiki Activity

Refactor homeserver lookup code #12

Merged
bad merged 10 commits from refactor-homeserver-lookup into princess 2020-10-23 08:57:01 +00:00
Conversation 0 Commits 10 Files changed 1 +6 -11
1 changed files with 6 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 16de7edd19 - Show all commits

11
src/js/login.js
View file

@ -71,7 +71,6 @@ class Form extends ElemJS {
this.on("submit", this.submit.bind(this)) this.on("submit", this.submit.bind(this))
} }
async submit() { async submit() {
if (this.processing) return if (this.processing) return
this.processing = true this.processing = true
@ -115,11 +114,11 @@ class Form extends ElemJS {
async findHomeserver(address, maxDepth = 5) { async findHomeserver(address, maxDepth = 5) {
//Protects us from servers sending us on a redirect loop //Protects from servers sending us on a redirect loop
maxDepth-- maxDepth--
if (maxDepth <= 0) throw new Error(`Failed to look up homeserver, maximum search depth reached`) if (maxDepth <= 0) throw new Error(`Failed to look up homeserver, maximum search depth reached`)
//Preprocess the address //Normalise the address
if (!address.match(/^https?:\/\//)) { if (!address.match(/^https?:\/\//)) {
console.warn(`${address} doesn't specify the protocol, assuming https`) console.warn(`${address} doesn't specify the protocol, assuming https`)
address = "https://" + address address = "https://" + address
@ -139,7 +138,7 @@ class Form extends ElemJS {
} }
/* jshint ignore:end */ /* jshint ignore:end */
// find the next matrix server in the chain // Find the next matrix server in the chain
bad marked this conversation as resolved Outdated
cadence commented 2020-10-22 04:45:27 +00:00
Outdated
Review
Copy link

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?
bad commented 2020-10-22 05:52:18 +00:00
Outdated
Review
Copy link

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
const root = await fetch(`${address}/.well-known/matrix/client`).then(res => res.json()).catch(e => { const root = await fetch(`${address}/.well-known/matrix/client`).then(res => res.json()).catch(e => {
console.error(e) console.error(e)
throw new Error(`Failed to look up server ${address}`) throw new Error(`Failed to look up server ${address}`)
@ -155,7 +154,6 @@ class Form extends ElemJS {
return this.findHomeserver(nextAddress, maxDepth) return this.findHomeserver(nextAddress, maxDepth)
} }
status(message) { status(message) {
feedback.setLoading(true) feedback.setLoading(true)
feedback.message(message) feedback.message(message)
@ -169,6 +167,3 @@ class Form extends ElemJS {
} }
const form = new Form() const form = new Form()