Merge pull request #681 from namecheap/fix/fix_fastify_callback

fix: fix fastify hook callback handling to prevent request hanging

Author: m2broth

ilc/server/app.js

@@ -31,23 +31,25 @@ module.exports = (registryService, pluginManager, context) => {
 
     app.addHook('onRequest', (req, reply, done) => {
         context.run({ request: req, requestId: reportingPluginManager.getRequestId() ?? request.id }, async () => {
+            let doneWithContext;
+
             try {
                 const asyncResource = new AsyncResource('fastify-request-context');
                 req[asyncResourceSymbol] = asyncResource;
-                const doneWithContext = () => asyncResource.runInAsyncScope(done, req.raw);
+                doneWithContext = () => asyncResource.runInAsyncScope(done, req.raw);
 
                 const { url, method } = req.raw;
                 accessLogger.logRequest();
 
                 if (!['GET', 'OPTIONS', 'HEAD'].includes(method)) {
                     logger.warn(`Request method ${method} is not allowed for url ${url}`);
                     reply.code(405).send({ message: 'Method Not Allowed' });
-                    return;
+                    return doneWithContext();
                 }
 
                 if (isDataUri(url)) {
                     reply.code(400).send({ message: 'Bad Request: Data URIs are not valid HTTP paths' });
-                    return;
+                    return doneWithContext();
                 }
 
                 req.raw.ilcState = {};
@@ -67,9 +69,15 @@ module.exports = (registryService, pluginManager, context) => {
 
                 await i18nOnRequest(req, reply);
 
-                doneWithContext();
+                return doneWithContext();
             } catch (error) {
                 errorHandler.handleError(error, req, reply);
+
+                if (typeof doneWithContext === 'function') {
+                    doneWithContext();
+                } else {
+                    done();
+                }
             }
         });
     });

ilc/server/app.spec.js

@@ -93,6 +93,25 @@ describe('App', () => {
         chai.expect(responseCombined2.body.message).to.include('Data URIs');
     });
 
+    it('should block data URI in onRequest hook before wildcardRequestHandler (prevents FragmentError)', async () => {
+        // Production NewRelic alert scenario: data URI reached wildcardRequestHandler -> special:404 -> FragmentError
+        // This test verifies the fix: data URI blocked in onRequest with 400, never reaches wildcardRequestHandler
+        const productionDataUri =
+            '/data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiPz48c3ZnIGlkPSJiIiB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSIxMTQ0IiBoZWlnaHQ9IjY1NSI';
+
+        const response = await server.get(productionDataUri).expect(400);
+
+        // Verify: Blocked with 400 (not 404 which would indicate wildcardRequestHandler was reached)
+        chai.expect(response.status).to.equal(400);
+        chai.expect(response.body.message).to.include('Bad Request');
+        chai.expect(response.body.message).to.include('Data URIs');
+
+        // Key assertion: Status is 400, not 404
+        // If this were 404, it would mean wildcardRequestHandler processed it as special:404
+        // which would trigger Tailor fragment fetching -> FragmentError in NewRelic
+        chai.expect(response.status).to.not.equal(404);
+    });
+
     it('should parse "invalid" urls', async () => {
         await server.get('///').expect(200);
     });