Skip to content

Commit

Permalink
fix some tests and code according to the linter rules (#62)
Browse files Browse the repository at this point in the history 
* fix: code according lint rules

* fix: tests
  • Loading branch information
@ceelsoin
ceelsoin authored Feb 29, 2024
1 parent 56116b8 commit c61d581
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 82 deletions.
13 changes: 6 additions & 7 deletions lib/domain/ErrorTracker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function getSandboxModules() {
function logSentryPublishError(ctx, publishErr, layer = '') {
const { event_id, tags } = ctx;
const { codeHash } = tags;
const { reason, statusCode, message, code, requestTime} = publishErr;
const { reason, statusCode, message, code, requestTime } = publishErr;

const prettyPublishError = {
reason,
Expand All @@ -56,10 +56,9 @@ function logSentryPublishError(ctx, publishErr, layer = '') {
requestTime,
codeHash,
};


if(prettyPublishError.hostname.private_key)
delete prettyPublishError.hostname['private_key']
if (prettyPublishError.hostname.private_key) {
delete prettyPublishError.hostname.private_key;
}

log.error('Failed to publish error into %s sentry', layer, prettyPublishError);
}
Expand All @@ -70,10 +69,10 @@ function notifySentryGlobal(ctx) {
return;
}
ctx.event_id = raven.generateEventId();
const timeStart = Date.now()
const timeStart = Date.now();
raven.send(ctx, (publishErr) => {
if (publishErr) {
publishErr.requestTime = Date.now() - timeStart
publishErr.requestTime = Date.now() - timeStart;
logSentryPublishError(ctx, publishErr, 'global');
return;
}
Expand Down
28 changes: 15 additions & 13 deletions lib/domain/Pipeline.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class Pipeline {
try {
const runScriptMetric = new Metric('run-script');
const result = await this.sandbox.runScript(step.script, this.req, options);
runScriptMetric.observeFunctionRunScript({ namespace, id });
runScriptMetric.observeFunctionRunScript({ namespace: step.namespace, id: step.id });

const spent = metric.observeFunctionRun({
namespace: step.namespace,
Expand All @@ -65,11 +65,11 @@ class Pipeline {
method: this.req.method,
request_uri: this.req.url,
status: result.status,
request_id: this.req.headers["x-request-id"] || "",
request_id: this.req.headers['x-request-id'] || '',
request_time: spent,
namespace: step.namespace,
function_id: step.id,
})
});

const previousResult = this.previousResult;
this.previousResult = result;
Expand Down Expand Up @@ -104,15 +104,17 @@ class Pipeline {
status,
});

log.info({
method: this.req.method,
request_uri: this.req.url,
status: status,
request_id: this.req.headers["x-request-id"] || "",
request_time: spent,
namespace: step.namespace,
function_id: step.id,
})
if (err.constructor.name === 'SandboxInternalException') {
log.info({
method: this.req.method,
request_uri: this.req.url,
status,
request_id: this.req.headers['x-request-id'] || '',
request_time: spent,
namespace: step.namespace,
function_id: step.id,
});
}

logStorage.console.error(`Failed to run function: ${err.message}, ${spent} seconds`);
logStorage.console.error(err.stack);
Expand All @@ -139,7 +141,7 @@ class Pipeline {

err.details = {
message: `Got error when execute pipeline function ${step.namespace}/${step.id}`,
}
};

throw err;
}
Expand Down
5 changes: 3 additions & 2 deletions lib/http/SchemaResponse.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ class SchemaResponse {

removeNotAllowedFields(data) {
const { env } = data;
if(env){
const fieldsToRemove = Object.keys(env).filter(field => this.notAllowedFields.includes(field));
if (env) {
const fieldsToRemove = Object.keys(env)
.filter(field => this.notAllowedFields.includes(field));
for (const field of fieldsToRemove) {
delete data.env[field];
}
Expand Down
53 changes: 19 additions & 34 deletions lib/http/routers/FunctionsRouter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const SchemaResponse = require('../SchemaResponse');
const router = new Router();
const { bodyParserLimit } = require('../../support/config');
const { reportError } = require('../../support/tracing');
const { RecordOtelError } = require('../../support/opentelemetry')
const { RecordOtelError } = require('../../support/opentelemetry');

function codeFileName(namespace, codeId) {
return `${namespace}/${codeId}.js`;
Expand Down Expand Up @@ -200,7 +200,7 @@ router.delete('/:namespace/:id', async (req, res, next) => {
});


router.all('/:namespace/:id/run', bodyParser.json({ limit: bodyParserLimit }), async (req, res, next) => {
router.all('/:namespace/:id/run', bodyParser.json({ limit: bodyParserLimit }), async (req, res) => {
const { namespace, id } = req.params;
const memoryStorage = req.app.get('memoryStorage');
const sandbox = req.app.get('sandbox');
Expand Down Expand Up @@ -234,20 +234,20 @@ router.all('/:namespace/:id/run', bodyParser.json({ limit: bodyParserLimit }), a
} catch (err) {
reportError(span, err);

const status = err.statusCode || 500
const status = err.statusCode || 500;
res.status(status).json({ error: err.message });

if(err.constructor.name === "SandboxInternalException"){
if (err.constructor.name === 'SandboxInternalException') {
log.error({
method: req.method,
request_uri: req.url,
status: status,
request_id: req.headers["x-request-id"] || "",
status,
request_id: req.headers['x-request-id'] || '',
message: err.message,
namespace: namespace,
namespace,
function_id: id,
stack: err.stack
})
stack: err.stack,
});
}

return;
Expand Down Expand Up @@ -285,19 +285,18 @@ router.all('/:namespace/:id/run', bodyParser.json({ limit: bodyParserLimit }), a
res.status(status).json({ error: err.message });

const spent = metric.observeFunctionRun({ namespace, id, status });

if(err.constructor.name === "SandboxInternalException"){
if (err.constructor.name === 'SandboxInternalException') {
log.error({
method: req.method,
request_uri: req.url,
status: status,
request_id: req.headers["x-request-id"] || "",
status,
request_id: req.headers['x-request-id'] || '',
message: err.message,
request_time: spent,
namespace: namespace,
namespace,
function_id: id,
stack: err.stack
})
stack: err.stack,
});
}

const logResult = logStorage.flush({
Expand All @@ -316,7 +315,7 @@ router.all('/:namespace/:id/run', bodyParser.json({ limit: bodyParserLimit }), a
code: code.code,
});
errTracker.notify(err);
RecordOtelError(err)
RecordOtelError(err); // eslint-disable-line
}
});

Expand Down Expand Up @@ -375,26 +374,12 @@ router.put('/pipeline', bodyParser.json({ limit: bodyParserLimit }), async (req,
res.json(result.body);
} catch (err) {
reportError(span, err);
RecordOtelError(err)
RecordOtelError(err); // eslint-disable-line
const status = err.statusCode || 500;

if(err.constructor.name === "SandboxInternalException"){
log.error({
method: req.method,
request_uri: req.url,
status: status,
spent: spent,
request_id: req.headers["x-request-id"] || "",
message: err.message,
namespace: step.namespace,
function_id: step.id,
stack: err.stack
})
}

metric.observePipelineRun(status);
const errDetails = { details: err.details ? err.details : {} }
res.status(status).json({ error: err.message, ...errDetails });
const errDetails = { details: err.details ? err.details : {} };
res.status(status).json({ error: err.message, ...errDetails });
}
next();
});
Expand Down
2 changes: 1 addition & 1 deletion lib/http/routes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ morgan.token('response-sectime', (req, res) => {
return secs.toFixed(3);
});
const app = express();
const traceEngine = config.trace.engine
const traceEngine = config.trace.engine;
app.use(morgan(config.log.morganFormat));
app.use(expressOpentracing.default({ tracer }));

Expand Down
2 changes: 1 addition & 1 deletion lib/support/config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const DEFAULT_GLOBAL_MODULES = [
];

module.exports = {
host: process.env.HOSTNAME || "localhost",
host: process.env.HOSTNAME || 'localhost',
port: ConfigDiscovery.getInt('PORT', 8100),
metricsPort: ConfigDiscovery.getInt('METRICS_PORT', 8101),
useNodeCluster: ConfigDiscovery.getBool('USE_NODE_CLUSTER', true),
Expand Down
43 changes: 23 additions & 20 deletions lib/support/opentelemetry.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,27 @@ const { ExpressInstrumentation } = require('@opentelemetry/instrumentation-expre
const { registerInstrumentations } = require('@opentelemetry/instrumentation');
const { CollectorTraceExporter } = require('@opentelemetry/exporter-collector-grpc');
const { NodeTracerProvider } = require('@opentelemetry/node');
const { IORedisInstrumentation } = require('@opentelemetry/instrumentation-ioredis')
const config = require('./config')
const { IORedisInstrumentation } = require('@opentelemetry/instrumentation-ioredis');
const config = require('./config');

const opentelemetry = require('@opentelemetry/api');

const OtelConfig = config.trace.otel;
const HOST = config.host
const HOST = config.host;

const collectorOptions = {
serviceName: OtelConfig.service,
url: `${OtelConfig.collector.host}:${OtelConfig.collector.port}`,
concurrencyLimit: 10
}
serviceName: OtelConfig.service,
url: `${OtelConfig.collector.host}:${OtelConfig.collector.port}`,
concurrencyLimit: 10,
};

const provider = new NodeTracerProvider();

provider.resource = provider.resource.merge({
attributes: {
"service.instance.id": HOST
}
})
'service.instance.id': HOST,
},
});

const exporter = new CollectorTraceExporter(collectorOptions);
provider.addSpanProcessor(new BatchSpanProcessor(exporter, {
Expand All @@ -39,21 +40,23 @@ registerInstrumentations({
span.updateName(`${attrs['http.method']} ${attrs['http.target']}`);
span.setAttribute('functions.route', attrs['http.route']);
span.setAttribute('functions.url', attrs['http.url']);
},
},
ignoreOutgoingUrls: [/.*\/agent_listener/, /.*\/sampling/],
ignoreIncomingPaths: [/.*\/healthcheck/, /.*\/metrics/, /.*\/sampling/]
ignoreIncomingPaths: [/.*\/healthcheck/, /.*\/metrics/, /.*\/sampling/],
}),
new ExpressInstrumentation()
new ExpressInstrumentation(),
],
});

provider.register();
const tracer = opentelemetry.trace.getTracer(OtelConfig.service);

function StartSpanMiddleware(req, res, next) {
const span = tracer.startSpan(req.path, { attributes: {
"http.request_id": req.headers["x-request-id"]
}});
const span = tracer.startSpan(req.path, {
attributes: {
'http.request_id': req.headers['x-request-id'],
},
});
req.otelSpan = span;
next();
}
Expand All @@ -65,10 +68,10 @@ function FinalizeSpanMiddleware(req, res, next) {
}

function RecordOtelError(err) {
const span = tracer.startSpan('Exception Throwed')
span.recordException(err.stack)
span.setStatus({code: opentelemetry.SpanStatusCode.ERROR })
span.end()
const span = tracer.startSpan('Exception Throwed');
span.recordException(err.stack);
span.setStatus({ code: opentelemetry.SpanStatusCode.ERROR });
span.end();
}

module.exports = { StartSpanMiddleware, FinalizeSpanMiddleware, RecordOtelError };
4 changes: 4 additions & 0 deletions test/fakes/FakeStorage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ class FakeStorage extends Storage {
return {
hash: 'my-hash-123',
code: 'function main() {}',
env: {
MY_VAR: 'MY_VALUE',
CLIENT_SECRET: 'FOOBAR',
},
};
}

Expand Down
3 changes: 3 additions & 0 deletions test/unit/domain/Pipeline.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ describe('Pipeline', () => {
beforeEach(() => {
req = {
body: {},
headers: {
'x-request-id': 'foobar',
},
};
sandbox = new Sandbox({});
step200 = {
Expand Down
8 changes: 4 additions & 4 deletions test/unit/http/routers/FunctionsRouter.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ describe('PUT /functions/pipeline', () => {
request(routes)
.put('/functions/pipeline?steps[0]=backstage/not-found')
.expect(404, {
error: 'Code \'backstage/not-found\' is not found', details: {}
error: 'Code \'backstage/not-found\' is not found', details: {},
}, done);
});
});
Expand Down Expand Up @@ -382,15 +382,15 @@ describe('PUT /functions/:namespace/:id/env/:env', () => {
});
});

describe('when get called, not return secret variable', () => {
it('should create an enviroment variable', (done) => {
describe('when get called', () => {
it('should not return a secret variable', (done) => {
request(routes)
.get('/functions/backstage/correct')
.set('content-type', 'application/json')
.send()
.expect((res) => {
expect(res.body.env).to.be.eql({
MY_VAR: 'MY VALUE',
MY_VAR: 'MY_VALUE',
});
})
.expect(200, done);
Expand Down

0 comments on commit c61d581

Please sign in to comment.