Skip to content
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.

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

Context lost in an unhandled exception (or rejection) #48

Open
ecdeveloper opened this issue Oct 16, 2019 · 5 comments
Open

Context lost in an unhandled exception (or rejection) #48

ecdeveloper opened this issue Oct 16, 2019 · 5 comments

Comments

@ecdeveloper
Copy link

First of all - great module!

I was wondering if there's any workaround to get the context, in case of an unhandled exception (or rejection, in case of promises). Here's a sample code:

// cls.js
const cls = require('cls-hooked');
const clsNamespace = cls.createNamespace(process.getuid());

module.exports = {
  middleware: (req, res, next) => {
    run(() => {
      let someContextId = getTheIDSomehow();
      set('context-id', someContextId)
      next()
    }),
  ns: clsNamespace
}
// some-other-file.js
const ns = require('./cls').ns;

process.on('unhandledRejection', () => {
  // this will be undefined
  console.log(ns.get('context-id')); 
});
@oneverest
Copy link

oneverest commented Nov 28, 2019

express? you can use the express-async-errors to handle the unhandleRejection issue.

const express = require('express');
require("express-async-errors");
const uuid = require('uuid');
const createNamespace = require('cls-hooked').createNamespace;

const namespace = createNamespace('com.foo');
const app = express();

app.use((req, res, next) => {
  const tid = uuid.v4();

  namespace.bindEmitter(req);
  namespace.bindEmitter(res);
  
  namespace.run(() => {
    req.on('end', () => {
      printContext("End Event");
    });
    namespace.set("url", req.url);
    namespace.set("tid", tid);
    next();
  });
});

app.get('/normal', (req, res) => {
  printContext();
  res.send('ok');
});

app.get('/promise', async (req, res) => {
  await asyncOperation();
  printContext();
  res.send('ok');
});

app.get('/exception', (req, res) => {
  throw new Error('unhandle exception!');
});

app.get('/rejection', async (req, res) => {
  throw new Error("unhandle rejection!");
});

app.use((err, req, res, next) => {
  printContext("uncaughtException");
  throw err;
});

function printContext(scienaro) {
  const tid = namespace.get("tid");
  const url = namespace.get("url");
  console.log(`[${scienaro ? scienaro : url}]: ${tid}`);
}

function asyncOperation() {
  return new Promise((resolve, reject) => {
    printContext();
    setTimeout(() => {
      printContext();
      resolve();
    });
  });
}

process.on("uncaughtException", (err) => {
  console.trace(err);
});

process.on("unhandledRejection", (err) => {
  console.trace(err);
});

app.listen(8586);

@wesleytodd
Copy link

wesleytodd commented Nov 28, 2019

I would strongly recommend against this module (express-async-errors). Reaching deeply into the router like this is a bad idea, it will also not work in v5. The behavior you want is already added in v5 (pillarjs/router#47 & pillarjs/router@d2bce0c).

In express v4, a much better approach is to use something like https://github.com/express-promise-router/express-promise-router

All that said, this is a bit off topic for this issue which is specifically about usage of this module.

@oneverest
Copy link

Thanks for your advise :)

@bizob2828
Copy link

@ecdeveloper I hit this with a generic error being thrown so maybe it will help. But if you look in the code for run it adds the context to a "symbol" in the error and throws it back up the chain

  let context = this.createContext();
  this.enter(context);

  try {
    if (DEBUG_CLS_HOOKED) {
      const triggerId = async_hooks.triggerAsyncId();
      const asyncHooksCurrentId = async_hooks.executionAsyncId();
      const indentStr = ' '.repeat(this._indent < 0 ? 0 : this._indent);
      debug2(`${indentStr}CONTEXT-RUN BEGIN: (${this.name}) currentUid:${currentUid} triggerId:${triggerId} asyncHooksCurrentId:${asyncHooksCurrentId} len:${this._set.length} context:${util.inspect(context)}`);
    }
    fn(context);
    return context;
  } catch (exception) {
    if (exception) {
      exception[ERROR_SYMBOL] = context;```

so i referenced it with `err[ERROR_SYMBOL]`

@zhujun24
Copy link

I have write a tidy npm package http-request-context to do this, using async_hooks, u can try it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants