EmailTemplate.js not loading into application

[rickhallett]

I have got to the end of sub-chapter 'Transactional emails with AWS SES'. On server start, I am unable to get my mLab mongoDB to show evidence of a new email document. If I run the equivalent 4-end code available on the official builderbook repo, mLab gets the document, so I know the error is somewhere in my code.

I have used diff tools to check the difference between my repository and the official 4-end repository on what I thought to be crucial pages: EmailTemplate.js, app.js etc. I cannot find any problematic differences.

I have also wiped all node modules, deleted my yarn.lock/package.json, and re-installed everything with yarn using the package.json available in 4-end, to no avail.

If I make breaking changes to EmailTemplate.js, no errors show at compilation time, leading me to believe that the code isn't being loaded.

My repository can be found here, with the latest commit functioning as described above

I would very much appreciate any assistance on this, as I've dropped quite a few hours into it now. I am sure it is something tiny...

[tima101]

@WhiteFire0 Could you please clarify exact thing you are trying to do?

I am unable to get my mLab mongoDB to show evidence of a new email document.

Do you mean that your app does not insert new document into empty collection of DB?

---

Edit:
Try this:

• stop app
• delete entire EmailTemplate collection
• start app
• check if collection is created and it has one document with name welcome

[rickhallett]

As you say @tima101 , no new document is inserted into the database. I am able to use the 4-end code to do this successfully, but if I delete the collection and run with my codebase I cannot replicate the result.

I should clarify perhaps; by 'breaking changes' to EmailTemplate.js, I mean basic syntactical errors. If I try to execute a function that doesn't exist, it doesn't throw an error. This is not true within the 4-end version of the code, which predictably throws a breaking error. This led me to assume that EmailTemplate.js is not being compiled.

[tima101]

@WhiteFire0 I see, got it.

It has to be code then. I will look into it tomorrow.

[rickhallett]

Are there any particular techniques/tools you use when debugging this kind of stack? If you are able to share how you might go about it, perhaps I can learn something in the process and also save your time :)

[tima101]

@WhiteFire0 Sure! Thanks for asking and being proactive. So main hypothesis that this is code-related not configuration since code inside book does work.

I would def-ly add console.log() to server/models/EmailTemplate.js. Specifically to insertTemplates() function to make sure it is called. You can also add to other places to make sure code runs.

I would also delete collection and restart app to make sure collection is created (meaning Mongoose runs as expected but first document is not created).

Problem is either with Mongoose or insertTemplates().

Could be some typo elsewhere, for example in scripts of package.json.
If you use VS Editor, it has neat feature to compare 2 files, you can use it to quickly compare your repo files to 4-end files.
https://stackoverflow.com/questions/30139597/visual-studio-code-is-there-a-compare-feature-like-that-plugin-for-notepad

[rickhallett]

Ok, found the bug!

The problem was as I initially expected; with the code as it stands in the book at the end of Chapter 4: Testing with Jest... > Transactional emails with AWS SES, the book shows screenshots that prove the new EmailTemplate.js code inserts a new document into the mongoDB.

This won't occur unless import getEmailTemplate from './EmailTemplate'; is added to User.js. I am not sure why this is, as insertTemplates() within EmailTemplate.js is requesting action. I assume it is because the User model is actively called upon within the HOC aspects of the code, whereas at this stage of the book the EmailTemplate model is not. This changes in the final demonstration code at point Chapter 4: Testing with Jest... > Transactional emails with AWS SES > Update User model, where signInOrSignUp() calls upon getEmailTemplate(). Perhaps Next.js does not load any code that isn't hoisted directly into the Express.js server?

Either way, the only change that needs to be made is the inclusion of the import statement before the screenshots that show the database being updated with the new emailtemplate document, as far as I can tell.

[rickhallett]

As a side note, whilst hunting this I found that the yarn.lock file in 4-end had MANY differences with mine, despite being generated from the same package.json. Some of this seemed to have been caused by the use of npm over yarn. I have exclusively used Yarn so far, as recommended at the start of the book.

[tima101]

@WhiteFire0

because the User model is actively called upon within the HOC aspects of the code,

Not sure that this is true.

When user logs in, passport package populates req.user with user information and holds it in server memory until user logs out.

withAuth HOC uses req.user to fetch data, for example user.email.

It would very helpful if you can create repo for me to test, repo in which EmailTemplate.js has all necessary code and document does not get inserted and User.js does not call getEmailTemplate(). Would you please create such repo it? Please add proper .gitignore file to that repo.

[rickhallett]

I'm not 100% sure what you mean by adding a proper .gitignore file. My local repository ignores with the following rule set:

*~
*.swp
tmp/
npm-debug.log
.DS_Store

.build/*
.next
.vscode/
node_modules/
coverage/
.coverage
.env
.next

Do you mean uploading node_modules, .next and the .gitignore file itself to the remote repository so that you can see all of the code base? I ask this as you mentioned in another thread to make sure I had node_modules and .next within my gitignore, which I always do.

Given the time zone difference, I have included them in this branch of my repository, in the state described in OP. I have tested it and it does not load the new email template document into mongoDB.

[tima101]

@WhiteFire0 I suggested committing .gitignore to the repo you want me to look at. In this case, this repo: https://github.com/whitefire0/ArchitectAtlas

So when I clone your repo to my machine, I have proper .gitignore file.

Here is an example of .gitignore for main app in builderbook repo: https://github.com/builderbook/builderbook/blob/master/.gitignore

[rickhallett]

@tima101 Got it - I understand you now. I . have reconfigured my root .gitignore file: the link in my previous post now shows the repository in the state you requested

[YuriGor]

As I said in duplicate issue, I already use require instead of import everywhere in my code (to use jest widely), there is no difference to import or to require, you still need to include the module to execute it, node doesn't know anything about any file until it was referenced explicitly. So we still need to import or require EmailTemplate.js to make it work. For example in the User model.
After I added const getEmailTemplate = require('./EmailTemplate'); into models/User.js, everything started to work fine.
So book text should be fixed, to suggest include EmailTemplate into user first, and only then to try first tests.

As for me, I'd prefer to have some separate init method in EmailTemplate.
Doing some writes to DB just on require, without explicit request from master code smells bad.

May be it's better to have some separate setup script, which will be called once on app startup.
And this script will call init nethod from EmailTemplate explicitly, and also will do all same tasks we will need later.

Imagine I write some unit test for User using clean mongoDB for testing(actually I do).
I require models/User, it requires EmailTemplates, and EmailTemplates instantly tries to write something to DB, before jest "beforeALL" called, where test mongo DB will be created in memory.

It looks like you get you tools case from a car, and drill start to work right inside the case, even before you opened it.

[tima101]

@WhiteFire0 I think @YuriGor provided good answer above. Did you try importing getEmailTemplate to User model?

I am gonna move testing of email templates in the book's content so it is located after this import is added to User model.

---

I tested importing getEmailTemplate to User model without using getEmailTemplate() and EmailTemplate doc was added to DB after I started app.

[YuriGor]

Same issue in Chapter 5 with testing Book/Chapter models - collections and indexes will not be created until we import/reqiure the models somewhere in actually running scripts.

Time to test. When we start our app, it should automatically creates indices, which will show up on our mLab dashboard.

Start your app with yarn dev.

Navigate to mLab, find the list of indices for Chapter collection.

[tima101]

@YuriGor @WhiteFire0

Added this blurb to Chapter 4:

"Before we test, open server/models/User.js and import getEmailTemplate:

import getEmailTemplate from './EmailTemplate';

No need to use getEmailTemplate any where inside server/models/User.js at this point. We added import because Node will execute code inside server/models/EmailTemplate.js only when this module is referenced explicitly. For more information, you read this issue."

[tima101]

@YuriGor @WhiteFire0

Added following blurb to Chapter 5:

"Before we test, open server/app.js and import Chapter:

import Chapter from './models/Chapter';

No need to use Chapter any where inside server/app.js just yet. We added this import because Node will execute code inside server/models/Chapter.js only when this module is referenced explicitly. For more information, you read this issue."

---

Let me know if it looks good to you. Thanks to both of you!

[YuriGor]

Yeah, looks good, now that readers, who follow book instructions, will not be confused.
May be you even don't need to add link to this issue:
it's expected behavior that node does nothing with files if nobody asked to do so,
so in this issue reader will only be surprised, why so obvious things caused so long discussion.
It's more a question of sync of book text and code, not a programming itself.

[rickhallett]

I strongly disagree that this is an obvious issue to people who are learning how Node behaves. My understanding is that it is an instructional book that aims to work for people with a variety of experience levels. There is nothing obvious about Node architecture to someone who doesn’t know it and the same is true for all programming.

[YuriGor]

IMHO this explanation

No need to use Chapter any where inside server/app.js just yet. We added this import because Node will execute code inside server/models/***.js only when this module is referenced explicitly.

is enough.
Link to this issue will add nothing valuable, not for newbie nor for experienced reader

[tima101]

@WhiteFire0 @YuriGor We try to provide both explanation and link in most cases, so let's keep both. "Obvious" is very relative term. Thanks.

[realtebo]

Thanks for poiting to this issue and for trying to help newbies like me.

We added import because Node will execute code inside server/models/EmailTemplate.js only when this module is referenced explicitly

I thinks it's a very bad piece of code. Sorry.
You must be sure that some code is executed at least one time. Ok.
But creating a old-style-procedural-like file and then importing only to force node to execute it is very sad. In fact, the linter is asking me why i'm importing it...

I've some coworkers doing same things in php with 'imports' only to avoid a small step of setup predeploy, so a useless piece of code is executed at every run.

Please, correct me if I have completely misunderstood the behaviours, the goal, the code, or your intention.

If, not so possibile, I am right, it's a bad code to show to new entries.

[tima101]

@realtebo Thank you for your passionate comment.

@YuriGor Curiousity question for an expert like you.

I thinks it's a very bad piece of code. Sorry.
You must be sure that some code is executed at least one time. Ok.
But creating a old-style-procedural-like file and then importing only to force node to execute it is very sad. In fact, the linter is asking me why i'm importing it...

Is this possible to achieve? Making code execute in module without referencing that module. Thanks.

[YuriGor]

Well, if we need to execute some code from the module - we will need to import it.
But let me quote my comment from above discussion:

As for me, I'd prefer to have some separate init method in EmailTemplate.
Doing some writes to DB just on require, without explicit request from master code smells bad.

May be it's better to have some separate setup script, which will be called once on app startup.
And this script will call init method from EmailTemplate explicitly, and also will do all same tasks we will need on app startup.

So as I got it - @realtebo is talking about the same things.
require, import, include and other dependency fulfillment operations should be passive.
Module is like a library with some tools.
It's ok to do some isolated self-preparation tasks on import, since node will actually do this only very first time, but fact of external data modification, that happens just on referencing, without explicit request from master code is source of potential problems with maintenance and reusability.

Now we hide this important initialize step behind the chain of dependencies:
app,js -> google.js -> models/User.js -> models/EmailTemplate.js -> implicit insert into DB

So change in or re-positioning of any of this chain's link can cause our email notifications to stop working,
And good chances are we will not even know about this, because users will not expect emails - so they will not report a problem, and there is no test in the project that covers this case.
And when we will figure out emails are broken, we will need to research and unwind all this chain (Nobody will remember all this implementation details, or bug will not happen otherwise)

It's much more clear and safe to reduce this chain to
app.js -> appInit.js -> models/EmailTemplate.js -> explicit insert into DB

ps now we have no much init tasks, so we can do things simpler
app.js -> models/EmailTemplate.js -> explicit insert into DB

[tima101]

@YuriGor @realtebo Thanks for explaining.

I think I get Yuri's explanation. Thanks for writing details. To make sure - do you guys suggest calling insertTemplates() via importing it to server/app.js (calling insertTemplates() explicitly). Right?

If so, I am open to such change.

[YuriGor]

Right, and remove implicit call from the depth of EmailTemplate

[realtebo]

I'm more to have a deploy script that insert the template

[YuriGor]

In the Chapter 4 there is no such thing as "deployment".
Deployment is subject of last chapter 8.

Running server/app.js in normal situation is equivalent to running post-deployment script:
it will start once after deployment and will be restarted again only in case of app crush or server reboot,
so there is no overhead to care about.

[tima101]

@YuriGor Would you like to create a pull request?

• export insertTemplates from EmailTemplate,js
• import insertTemplates to server/app.js
• call it at this location: https://github.com/builderbook/builderbook/blob/master/book/4-end/server/app.js#L52

The code that needs to be modified is in following subdirectories (builderbook/book/*):

• 4-end
• 5-start, 5-end
• 6-start, 6-end
• 7-start, 7-end
• 8-start, 8-end

Once code is updated, I will edit book's content to sync it with code.

[YuriGor]

Ok, but not instantly Best, Yuri

…

________________________________ From: Tima <[email protected]> Sent: Thursday, November 15, 2018 8:20:17 PM To: builderbook/builderbook Cc: YuriGor; Mention Subject: Re: [builderbook/builderbook] EmailTemplate.js not loading into application (#111) Reopened #111<#111>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#111 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AGiyBfZ4zZC8MKuAzfW-O3p-tu_wkQb5ks5uvaJRgaJpZM4Wynk3>.

[tima101]

@YuriGor Yeah, this is not urgent at all. Thanks.

[YuriGor]

  templates.forEach(async (template) => {
    if ((await EmailTemplate.find({ name: template.name }).countDocuments()) > 0) {
      return;
    }

    EmailTemplate
      .create(template)
      .catch((error) => {
        logger.error('EmailTemplate insertion error:', error);
      });
  });

• Lets always upsert templates - if there are no templates, they will be inserted, if they exist - they will be updated, so in case of changes in the templates we will not need to go and empty collection manually. This method runs once per app start, even if there will be no changes most of time, couple of updates costs nothing.
• Why swallow exception here? If we failed to insert into db, then something is very wrong, does it make sense to start an app in such case? Nobody will see that log messages if app was started anyway.

I suggest this code instead:

templates.forEach(async (template) => {
   try {
     await EmailTemplate.updateOne({ name: template.name }, template, { upsert: true });
   } catch (error) {
     logger.error('EmailTemplate insertion error:', error);
     throw error;
   }
 });

[tima101]

@YuriGor upsert looks good to me. I would need write explanation in the book. It's ok by me.

Do you still want to do following?

• export insertTemplates from EmailTemplate,js
• import insertTemplates to server/app.js
• call it at this location: https://github.com/builderbook/builderbook/blob/master/book/4-end/server/app.js#L52

[YuriGor]

Yes, I already did this for chapter 4-end(not committed yet) and tested, it works fine, but before I'll copy this code over other chapters I wanted you to confirm this changes. So now, since you agree, I still need to do this for the rest of chapters, it will take some time to test.

[tima101]

@YuriGor Thanks! You can do just one chapter, Kelly or I will propagate the change to other chapters.

[YuriGor]

Ok, lets finish with #169 and I'll push this one.

[YuriGor]

I pushed this changes into separate branch in my repo, so you can start code review even if there is still no PR.
And when #170 will be merged, I'll create new one from this branch

[tima101]

Note to self: we are waiting for #180

---

@YuriGor Thank you for creating PR.
I reviewed changes: https://github.com/builderbook/builderbook/pull/180/files

A small comments: would you consider adding try/catch around await insertTemplates();?

To make life easier for us and reader, do you mind @YuriGor if I simply mention your approach in the text in addition to other solution (importing to server/models/User.js)?
And not merge this PR into actual code of Chapter 4?

[YuriGor]

Hi @tima101, I've got no notification about you mentioned me in your recent comment
(may be github does not send notifications about edits?)

Now I see you merged my PR, so is your suggestion about try/catch still actual?
If so - what exactly do you suggest to do in the catch block?
I think it's better to not swallow an exception here and to let it bubble up, because if something wrong happened during attempt to simple DB write - there is no so much sense to even try start the app.
It's better app will not start with error so developer/admin will not miss the fact of error, so he will not have running app with broken email notifications without even noticing it.

About second question - it's unclear for me what are you talking about :)
I don't remember exactly which phase of the app development was reflected in any of chapters, so
please manage the flow of the book story as you think it will be better.

[tima101]

@YuriGor Thanks for your reply. Got it.

Pushed changes to all chapters and main app: c##111 main app and all book chapters

---

Fixed code and text.