j3k0 / ganomede-users

Ganomede Users
0 stars 0 forks source link

sendPasswordResetEmail error code #36

Open j3k0 opened 7 years ago

j3k0 commented 7 years ago

When sendPasswordResetEmail is given an unknown email address, we can use a more specific error instead of DocumentNotFoundError: EmailNotFoundError (which already defined somewhere).

j3k0 commented 7 years ago

Additionally, this shouldn't be logged as an error: some user don't remember his email address? It just needs an info.

[2017-03-24T15:14:52.909Z] ERROR: users/16 on 48e2841e53bf: Document not found {"_id":"alias:email:jxxx"} (req_id=3bf1b606-15c0-4a3f-81d4-7065dc19e8c2)
    DocumentNotFoundError: Document not found {"_id":"alias:email:jxxx"}
        at parseResponse (/home/app/code/node_modules/restify/lib/clients/json_client.js:71:23)
        at IncomingMessage.done (/home/app/code/node_modules/restify/lib/clients/string_client.js:165:13)
        at IncomingMessage.g (events.js:291:16)
        at emitNone (events.js:91:20)
        at IncomingMessage.emit (events.js:185:7)
        at IncomingMessage.wrapped (/home/app/code/node_modules/newrelic/lib/transaction/tracer/index.js:184:28)
        at IncomingMessage.wrappedResponseEmit [as emit] (/home/app/code/node_modules/newrelic/lib/transaction/tracer/instrumentation/outbound.js:99:26)
        at endReadableNT (_stream_readable.js:974:12)
        at wrapped (/home/app/code/node_modules/newrelic/lib/transaction/tracer/index.js:184:28)
        at _combinedTickCallback (internal/process/next_tick.js:80:11)
        at process._tickDomainCallback [as _tickCallback] (internal/process/next_tick.js:128:9)
j3k0 commented 7 years ago

sendPasswordResetEmail's EmailNotFoundError` is only implemented in the stormpath backend, needs to be on the ganomede-directory backend. Also there's the logging level issue.

elmigranto commented 7 years ago

Error is sent from directory, so we need to change it there. Working on it.

elmigranto commented 7 years ago

Uhm, email is alias, so it goes here:

byAlias (type, value, callback) {
    async.waterfall([
      (cb) => this.db.get(`alias:${type}:${value}`, cb),
      (doc, cb) => {
        return doc
          ? this.byUserId(doc.id, cb)
          : cb(new Error('NotFound'));
      }
    ], callback);
  }

This code is a bit wrong, since db.get will error on couch 404. We can change it to nullableGet, double check stuff and maybe replace error message to ${capitalize(type)}NotFound or leave it as DocumentNotFoundError, but use error message to specify details. Though, latter would require client changes, since my understaning is that it simply shows error's name.

elmigranto commented 7 years ago

Ref https://github.com/j3k0/ganomede-directory/pull/34