tl-its-umich-edu / canvas-course-manager-next

Canvas Course Manager Next: A redesign of the existing CCM application. It extends Canvas features, makes cumbersome features easier to use, and adds new features.
8 stars 9 forks source link

Add root route and database health checks using `@nestjs/terminus` (#343) #345

Closed ssciolla closed 2 years ago

ssciolla commented 2 years ago

This PR aims to resolve #343.

Resource(s):

ssciolla commented 2 years ago

I'm not sure whether the Codacy alerts are legit, or whether they're just part of the same issues related to typing with libraries that we've seen. Anyways, I don't see any problems locally in VSCode.

pushyamig commented 2 years ago

I agree with you. VSCode don’t show error but Codacy does. I get confused where the problem is. I will go by our Configurations

On Thu, Feb 17, 2022 at 4:52 PM Sam Sciolla @.***> wrote:

I'm not sure whether the Codacy alerts are legit, or whether they're just part of the same issues related to typing with libraries that we've seen. Anyways, I don't see any problems locally in VSCode.

— Reply to this email directly, view it on GitHub https://github.com/tl-its-umich-edu/canvas-course-manager-next/pull/345#issuecomment-1043495338, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBOVP6FUJDXH77OP7PKSOTU3VU3FANCNFSM5OV4BCJQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because your review was requested.Message ID: @.*** com>

-- Regards, Pushyami

Need help with online instruction? Join the Teaching Online with Canvas Workshops: From Good to Great! https://its.umich.edu/training/canvas | Workshop Video Playlist https://www.mivideo.it.umich.edu/playlist/dedicated/137537371/1_bc51ybxg/1_fart74iy

pushyamig commented 2 years ago

@ssciolla At this point I feel we can create a separate issue when DB is shutdown why the error is throws instead of stating up/down. At this moment I don't think we can do much if the library is not supporting it. The integration is working as per the recipes by the Author.
I think we can merge this issue and get the Nagios configuration process by Rick.

ssciolla commented 2 years ago

@pushyamig, we can merge this if you want to. I'm still seeing bizarre behavior though. Specifically, I'm seeing errors like this one when I try to hit the endpoint while the database service is down or mysql is in offline_mode (use SET GLOBAL offline_mode=1;), which are often masking a SequelizeConnectionError:

Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
    at new NodeError (node:internal/errors:371:5)
    at ServerResponse.setHeader (node:_http_outgoing:576:11)
    at ServerResponse.header (/base/node_modules/express/lib/response.js:771:10)
    at ServerResponse.send (/base/node_modules/express/lib/response.js:170:12)
    at ServerResponse.json (/base/node_modules/express/lib/response.js:267:15)
    at ExpressAdapter.reply (/base/node_modules/@nestjs/platform-express/adapters/express-adapter.js:36:57)
    at CSRFExceptionFilter.handleUnknownError (/base/node_modules/@nestjs/core/exceptions/base-exception-filter.js:38:24)
    at CSRFExceptionFilter.catch (/base/node_modules/@nestjs/core/exceptions/base-exception-filter.js:17:25)
    at CSRFExceptionFilter.catch (/base/server/src/auth/csrf.exception.filter.ts:32:18)
    at ExceptionsHandler.invokeCustomFilters (/base/node_modules/@nestjs/core/exceptions/exceptions-handler.js:33:26) {
  code: 'ERR_HTTP_HEADERS_SENT'

This might be a bug somewhere else in the application, but I haven't been able to figure out where.

That said, I was able to reproduce this on the main branch, so it's not a new issue created by this PR.

pushyamig commented 2 years ago

Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client

I sometime see this error when I am debugging in VSCode when I hit a break point and resume. I never saw this during the normal flow of execution.

I think we can merge this for now.