Skip to content

Initialize context from middleware and not on get#18

Merged
michaelkrone merged 1 commit intomichaelkrone:masterfrom
bookmd:fix-init-context
Jan 16, 2017
Merged

Initialize context from middleware and not on get#18
michaelkrone merged 1 commit intomichaelkrone:masterfrom
bookmd:fix-init-context

Conversation

@chenrozenes
Copy link
Copy Markdown
Contributor

When running the entire express app inside a domain context, the domain.active has value.
If we call get/getContext for the first time, it will initialize the ___$cntxt___ object before we created our request context domain.
In this situation the middleware will not create another domain, and the outer domain will be used for all requests.
The problematic flow is this:

// Running inside a domain context (aka domain X)

// Somewhere we do...
contextService.get('request');

// express's middleware declaration
app.use(contextService.middleware('request')) // Another domain hasn't been created

// inside an express route
contextService.set('request:userId', '123'); // value saved on domain X

The fix will initialize the context object only within the middleware, get and set calls will address the request-context domain only

@michaelkrone
Copy link
Copy Markdown
Owner

@chenrozenes
I did not look into this yet, but why the close?

@chenrozenes chenrozenes reopened this Dec 21, 2016
@chenrozenes
Copy link
Copy Markdown
Contributor Author

chenrozenes commented Dec 21, 2016

@michaelkrone I ran another test of mine that suddenly didn't work so I closed it until I'll figure it out, but all good

@michaelkrone
Copy link
Copy Markdown
Owner

OK, I will look into this tonight and publish a new version if it looks ok.

@chenrozenes
Copy link
Copy Markdown
Contributor Author

@michaelkrone Hey, have you checked it out?

@michaelkrone
Copy link
Copy Markdown
Owner

michaelkrone commented Jan 16, 2017

Sorry for the delay. Will publish 1.0.1 2.0.0 now.

@michaelkrone michaelkrone merged commit 62762b1 into michaelkrone:master Jan 16, 2017
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

Successfully merging this pull request may close these issues.

2 participants