Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Add an "echo" endpoint to the default app that responds with the details of the request #1668

Open
johnboxall opened this issue Feb 14, 2024 · 2 comments
Labels
Acknowledged Team has responded to issue

Comments

@johnboxall
Copy link
Collaborator

johnboxall commented Feb 14, 2024

CDNs often have quirks in terms of how they forward requests to origins.

When customers stack a CDN on MRT, we often debug request forwarding issues To debug issues with forwarding, we often suggest customers add an "echo" endpoint to ssr.js to their apps:

// ssr.js
app.all('/reflect', function echoHandler({method, path, headers, body}, res) {
  return res.send({method, path, headers, body})
})

It would be nice if we included this endpoint by default in apps with an environment variable toggle so folks could enable it without a code change.

This would simplify debugging many of these issues!

@bendvc bendvc added the Acknowledged Team has responded to issue label Feb 16, 2024
@bendvc
Copy link
Collaborator

bendvc commented Feb 16, 2024

Hi @johnboxall 👋

Sounds like a good idea. I'm wondering is something like this might be a little more future-proof.

// ssr.js

import {devMiddleware} from `pwa-kit-react-sdk`

...

if (!env.isProduction || env.enableServerDevEnpoint) {
    app.use(devMiddleware())
}

Here we can add your suggested reflect endpoint under a common /__dev path. This would allow use to add more end-points with out worrying about coming up with pathnams that might collide with user defined ones. Another idea might be /__dev/info that would give the user and us details about the version of sdk/node/etc they are using.

@johnboxall
Copy link
Collaborator Author

johnboxall commented Feb 16, 2024

@bendvc – your suggestion about not conflicting with existing customer routes is a good one!

Instead of introducing a new path, we could extend the current convention of /mobify/ping and friends with /mobify/reflect. Alternatively, we could even extend ping to conditionally provide this information.

One key part of my suggestion is that ideally this endpoint is available across all environments (including production ones!) and can be enabled without a code change – so I'm imagining something that is default off, but can be enabled using an MRT environment variable (eg. MRT_REFLECT_ENABLED=$TRUTHY)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Acknowledged Team has responded to issue
Projects
None yet
Development

No branches or pull requests

2 participants