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

Exception when using takeover response in onPreResponse and the server is configured with autoValue #4316

Open
sveisvei opened this issue Nov 11, 2021 · 4 comments
Labels
support Questions, discussions, and general support

Comments

@sveisvei
Copy link

Support plan

  • is this issue currently blocking your project? (yes/no): no
  • is this issue affecting a production system? (yes/no): no

Context

  • node version: v14.17.6
  • module version with issue: "@hapi/hapi": "^20.2.1",
  • last module version without issue: ?
  • environment (e.g. node, browser, native): node
  • used with standalone
  • any other relevant information:

What are you trying to achieve or the steps to reproduce?

I am using the cookie state autoValue for a persistent id.
When trying to use the onPreResponse to takeover a response, it fails because request.state is null.

TypeError: Cannot use 'in' operator to search for 'tid' in null
    at exports.state (/node_modules/.pnpm/@hapi+hapi@20.2.1/node_modules/@hapi/hapi/lib/headers.js:88:57)
    at Object.internals.marshal (/node_modules/.pnpm/@hapi+hapi@20.2.1/node_modules/@hapi/hapi/lib/transmit.js:41:15)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at async Object.exports.send (/node_modules/.pnpm/@hapi+hapi@20.2.1/node_modules/@hapi/hapi/lib/transmit.js:27:9)
    at async Request._reply (/node_modules/.pnpm/@hapi+hapi@20.2.1/node_modules/@hapi/hapi/lib/request.js:457:5)

Reproducible demo:

const Hapi = require("@hapi/hapi");
const { isBoom } = require('@hapi/boom');

const server = new Hapi.Server({
  port: 3333,
});

server.state("tid", {
  ttl: 1000 * 60 * 60 * 24 * 31 * 3, // 3 months
  isSecure: false,
  isHttpOnly: true,
  encoding: "none",
  clearInvalid: true,
  strictHeader: true,
  autoValue: () => 'v',
  path: "/",
});

server.ext("onPreResponse", (request, h) => {
  const err = request.response;
  if (isBoom(err)) {
    return h.response("test").code(err.output.statusCode);
  }
  return h.continue;
});

server.start();
console.log(server.info.uri);

What was the result you got?

Exception.

What result did you expect?

I would expect that onPreResponse can be taken over.

@sveisvei sveisvei added the support Questions, discussions, and general support label Nov 11, 2021
@sveisvei
Copy link
Author

Open to a PR to fix this? My naive PR would improve the sanity checks on states, but I assume the error really is in the actual states object MIA.

Alternative is for me to stop using the autoValue() feature.

@devinivy
Copy link
Member

Hey, thanks for the report. I will take a look at this some time today, but if you can see what's wrong we'd certainly accept a PR!

@sveisvei
Copy link
Author

I can have a look, but do you have an hunch why the state might be undefined when throwing an error inside onPreResponse?

@kanongil
Copy link
Contributor

This could very well be related to #4297, which provides some insight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
support Questions, discussions, and general support
Projects
None yet
Development

No branches or pull requests

3 participants