Opened 20 months ago

Closed 19 months ago

Last modified 6 weeks ago

#6549 closed change (rejected)

Silence node-sass output on success

Reported by: tlucas Assignee:
Priority: Unknown Milestone:
Module: User-Interface Keywords:
Cc: greiner, agiammarchi, saroyanm Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description (last modified by tlucas)

Environment

OS: Debian

  • npm @ 5.6.0
  • node @ v8.11.1
  • adblockpluschrome @ 8f44d27 (git) 5039d6d02b4c (hg)

Background

In #6309, we started using node-sass for bundling CSS styles in adblockplusui.
After fixing postinstall-dependencies in #6488 (first occurrence of this behavior), node-sass writes a rendering-completed-message to stderr:

Rendering Complete, saving .css file...
Wrote CSS to /home/tlucas/abp/gitrepo/adblockpluschrome/adblockplusui/skin/desktop-options.css

While this behavior was reproduced in the aforementioned environment,
it causes the Cron Daemon on abp-builds-1 (the buildserver) to dispatch an email, every time adblockpluschrome is rebuilt:

Subject: Cron <devbuilds@abp-builds-1> python -m sitescripts.extensions.bin.createNightlies 
Message:

Rendering Complete, saving .css file...
Wrote CSS to /tmp/abpchromeLxGrxR/adblockplusui/skin/desktop-options.css
Rendering Complete, saving .css file...
Wrote CSS to /tmp/abpfirefoxyn05v5/adblockplusui/skin/desktop-options.css

While it's valuable to be informed about errors happening while automatically building the extension, IMHO it is of no value to receive a rather meaningless "done"-email from cron.

What to change

Let node-sass only write to stderr when an actual errors happens, in order to not dispatch any unnecessary email from inside a cron-job on abp-builds-1.

Change History (8)

comment:1 Changed 20 months ago by tlucas

According to https://www.npmjs.com/package/node-sass , section "options", passing -q or --quiet should be the solution.

comment:2 Changed 20 months ago by agiammarchi

to be honest, I actually like while watching in development logs to see things are happening.

can we solve this using some environment variable instead of always disabling the sass output?

comment:3 follow-up: Changed 20 months ago by greiner

The issue appears to be that node-sass is writing this info message to stderr instead of stdout for whatever reason. Presumably, it does that because it wants to send the output to stdout if the --output flag is omitted.

comment:4 in reply to: ↑ 3 ; follow-up: Changed 20 months ago by agiammarchi

Replying to greiner:

node-sass is writing this info message to stderr instead of stdout for whatever reason.

sounds like a bug to me (a node-sass one)

Presumably, it does that because it wants to send the output to stdout if the --output flag is omitted.

outputting properly sounds good to me

comment:5 in reply to: ↑ 4 Changed 20 months ago by tlucas

  • Description modified (diff)

Replying to agiammarchi:

Replying to greiner:

node-sass is writing this info message to stderr instead of stdout for whatever reason.

sounds like a bug to me (a node-sass one)

Presumably, it does that because it wants to send the output to stdout if the --output flag is omitted.

outputting properly sounds good to me

Ah yes, my bad - changed the description accordingly.
How it's done in detail is up to the dev tackling this i guess - as long as nothing's printed to stderr, when no error happens ;)

Last edited 20 months ago by tlucas (previous) (diff)

comment:6 Changed 19 months ago by greiner

What about adding a wrapper script for calling node-sass (e.g. build/sass.js) so that we can forward its output to the appropriate channel?

Because the only option I could find that'd allow us to keep using the CLI is to use the --error-bell flag to differentiate errors in the output but I'd consider that to be quite a workaround.

comment:7 Changed 19 months ago by tlucas

  • Resolution set to rejected
  • Status changed from new to closed

We won't handle this on our side, instead ops will filter out the messages -> see http://hub.eyeo.com/issues/10639

comment:8 Changed 4 months ago by vedantydv123

spam

Last edited 6 weeks ago by kzar (previous) (diff)
Note: See TracTickets for help on using tickets.