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

18.0.0 Release Notes #3871

Closed
hueniverse opened this issue Nov 6, 2018 · 6 comments
Closed

18.0.0 Release Notes #3871

hueniverse opened this issue Nov 6, 2018 · 6 comments
Assignees
Labels
breaking changes Change that can breaking existing code release notes Major release documentation
Milestone

Comments

@hueniverse
Copy link
Contributor

hueniverse commented Nov 6, 2018

Summary

hapi v18.0.0 is a small size release with a few changes to improve performance and standards compliance.

  • Upgrade time: low - none to a couple of hours for most users
  • Complexity: medium - requires following the list of changes to verifying their impact
  • Risk: medium - small number of changes but the implementation details of the memory cache could impact large use cases which requires product testing
  • Dependencies: medium - existing plugins will work as-is, however logging plugins are likely to require minor tweaks

Breaking Changes

Note: changes marked with validated are validated by the framework and will fail to execute if not properly migrated. They are considered safe / easy changes since they will throw an error immediately.

  • server.options.cache and server.cache.provision() configuration restructured. validated
  • Upgrade catbox-memory to version 4.0.1 which changes the in-memory cache implementation from a timer per key to a single timer for the entire cache.
  • Replace the request URL processing from the deprecated Url.parse() method to the WHATWG URL API.
  • Change server.inject() authentication options. validated
  • Failed responses are now logged (instead of request.response being forced to null), including aborted/closed requests.
  • server.auth.test() return value changed from credentials to { credentials, artifacts }.
  • request.info.responded now only set when response transmitted successfully.

New Features

  • server.options.query.parser - new configuration to customize query parameters processing.
  • New memory cache configuration options: minCleanupIntervalMsec and cloneBuffersOnGet.
  • route.options.response.disconnectStatusCode - allows setting a default status code for disconnected client errors. Defaults to 499.
  • Cookie validation options route.options.validate.state as well as access to state inputs in other validations.

Bug fixes

Updated dependencies

  • boom from v7.2.2 to v7.3.3
  • bounce from v1.2.2 to v1.2.3
  • catbox from v10.0.5 to v10.0.6
  • catbox-memory from v3.1.3 to v4.0.1
  • hoek from v6.02 to v6.1.2
  • joi from v14.0.4 to v14.3.1
  • podium from v3.1.4 to v3.2.0
  • teamwork from v3.0.2 to v3.0.3
  • vise from v3.0.1 to v3.0.2

Migration Checklist

Server Cache Configuration

This release streamlines the configuration and provisioning of server caches. It was done to prevent confusion when configuring cache strategies and prevent deep cloning of options passed which prevent some combination from operating properly.

The engine property no longer supports passing a class of constructor function. Instead, a new provider configuration key was added. In addition, the partition option was moved into the new provider.options.

For example:

// Before:
server.cache.provision({ engine: require('catbox-memory'), name: 'countries' });

// After:

server.cache.provision({ provider: require('catbox-memory'), name: 'countries' });

// Before:
server.cache.provision({ engine: require('catbox-memory'), name: 'countries', partition: 'x', maxByteSize: 10000 });

// After:
server.cache.provision({
    provider: {
        constructor: require('catbox-memory'),
        options: {
            partition: 'x',
            maxByteSize: 10000
        }
    }
    name: 'countries'
});

Checklist:

  • Look for engine config references and where found, rework the config to match the new format.

In-Memory Cache

This release replaces the implementation of the catbox-memory cache which is the default cache implementation provided with every server instance. Before this change, every item stored in the cache created a system timer which fired when the item expired. This worked fine for small number of items but in some cases with many items, the timer count could become a problem. This was made significantly worse if node domains are used since they increase the memory footprint of each timer.

The new implementation uses a single timer which is only set when a value is stored in the cache. If the cache contains a large number of items with very small timeouts, the processing overhead may cause a different load pattern than before (repeated cache iterations vs. repeated timers management).

Note that the in memory cache was never designed to be a replacement for a proper cache such as Memcached or Redis. It is a useful tool for simple use cases. If you are using it in any meaningful scale, you should test how this change impacts your server's performance under load.

To help control the new behavior, the following new cache configuration option is available:

  • minCleanupIntervalMsec - the minimum number of milliseconds in between each cache cleanup. Defaults to 1 second (1000).

Checklist:

  • review any memory cache usage in your application and ensure that if they are of significant size, that the new implementation does not have negative impact on server performance.

WHATWG URL

Following node's move to deprecate its Url.parse() and Url.format() methods, this release migrates its URL format to use the new WHATWG URL object. Since most applications use the request.path, request.query, and request.info properties, those will remain unaffected.

However, for those accessing request.url, the following changes apply:

  • Previous properties that remain exactly the same:
    • request.url.pathname
    • request.url.search
  • Previous properties that will now always include values (before they were often null):
    • request.url.protocol
    • request.url.host
    • request.url.hostname
    • 'request.url.port'
    • request.url.hash
  • Previous properties that have different values:
    • request.url.href - new value is now always the full absolute URL (previously it was often relative URL)
  • Previous properties that are no longer supported:
    • request.url.slashes
    • request.url.auth - replaced by the new request.url.username and request.url.password properties
    • request.url.query - use request.query instead
    • request.url.path - use request.path or request.url.pathname
  • New properties:
    • request.url.origin
    • request.url.username
    • request.url.password
    • request.url.searchParams

When calling request.setUrl(url), the url argument must be one of:

  • absolute URL string
  • an instance of Url.URL

Previously it was possible to pass a Url.parse() object with a string query which caused request.query to be a string instead of an object. This is no longer a possible side effect. Passing the result of Url.parse() as url is no longer supported.

In onRequest extensions, if the request path is invalid, request.url will now be set to null and if it is not corrected via request.setUrl(), the request will result in a bad request error response. Previously, the request.url was set to best effort parsing which was mostly invalid.

Checklist:

  • look for request.url references in your code and make sure the changes to the available properties do not impact you (or adjust accordingly).
  • look for request.setUrl() references in your code and ensure you are only passing valid arguments.
  • if you use request.setUrl() to override query processing (e.g. using the qs module), consider switching to the much simpler server.options.query.parser option.
  • if you have onRequest extension handlers, make sure your code can handle request.url being set to null if the request.path received is invalid.

server.inject() Authentication

In order to fully support request injection authentication as a production-ready feature, the injection options were changed to require passing the name of the strategy being simulated to authenticate the request. Previously, a special 'bypass' strategy name was set for all injected credentials.

To use the new format simply wrap the credentials and optional artifacts with an auth object and add a new strategy key with a name matching a configured authentication strategy.

For example: await server.inject({ url: '/', credentials: { user: 'steve' } }); changes to await server.inject({ url: '/', auth: { credentials: { user: 'steve' }, strategy: 'default' } });.

Checklist:

  • Scan your code for server.inject with credentials option and adjust to the new format.

  • Scan your code for references to 'bypass' to ensure you are not relying on the fake strategy name in your code. If you need to know if a request has been injected with credentials, use request.auth.isInjected.

Response Errors

In previous versions, when a response failed to transmit fully, the error was logged as a request error but then the request.response value was forced to null. This had the side effect of causing logs to have no access to the actual status code and default to 200. It also made it more difficult to handle such errors via the response event since by then the request.response value was already forced to null.

In addition, request.info.responded was always set to a timestamp when request processing completed, regardless if a response was actually successfully transmitted. Instead, request.info.responded will now only be set to a non 0 value if the response was successfully sent. A new request.info.completed value will always contain a valid timestamp indicating when the request completed processing (right before the 'response' event is emitted).

Checklist:

  • Review how you handle request.response values when a request ends (e.g. the response event). If you currently check for null, check for errors instead. Internal transmit errors will use status code 500 and client disconnections will use status code 499 (which can be customized via route.options.response.disconnectStatusCode.

  • Consider updating your logging facilities to handle the change in how client disconnections are reported (from a default of 200 to the new 499 value). You may want to ignore 499 errors if you do not care about disconnections, or produce new reports based on the new data.

  • If you access request.info.responded (usually in logging logic), consider switching to request.info.completed if you want to know when the server finished processing and use the existence of request.info.responded to log if a response was successful.

server.auth.test() Return Value

The return value changed from plain credentials to { credentials, artifacts }. Simply replace: const credentials = server.test.auth(...) with const { credentials } = server.test.auth(...).

Checklist:

  • Scan your code for server.auth.test (or maybe just .auth.test in case you call the server object something else, and fix the assignment of the return value accordingly.
@hueniverse hueniverse added breaking changes Change that can breaking existing code release notes Major release documentation labels Nov 6, 2018
@hueniverse hueniverse added this to the 18.0.0 milestone Nov 6, 2018
@hueniverse hueniverse self-assigned this Nov 6, 2018
@hueniverse hueniverse changed the title 18.0.0 Release Notes [DRAFT] 18.0.0 Release Notes Nov 6, 2018
@hueniverse hueniverse changed the title [DRAFT] 18.0.0 Release Notes 18.0.0 Release Notes Jan 18, 2019
@kanongil
Copy link
Contributor

I looked into request.url support in the hapi modules, and found that h2o2 and hapi-auth-cookie use request.path, and will need to be updated for full compatibility.

Additionally, I found that the ignorePaths option in hapi-pino won't work. There are likely other more subtle issues as well.

Watch out if you use any of these modules.

mcollina pushed a commit to hapijs/hapi-pino that referenced this issue Feb 8, 2019
According to the hapi 18 release notes, request.url.path has
gone away in favor of request.path or request.url.pathname.
The same release notes also state that request.url.pathname
has not changed.

Refs: hapijs/hapi#3871
@rluba
Copy link
Contributor

rluba commented Feb 19, 2019

@hueniverse request.url.path included the query param string! Therefore the migration guide for request.url.path should read:

request.url.path: use request.url.pathname + request.url.search instead

… to avoid subtle mistakes during migration.

@framerate
Copy link

Has anyone confirmed Bell working with v18? I was having trouble and was going to try and investigate this week. Possibly related to this behavior: hapijs/bell#394

@ioanlucut
Copy link

@rluba we just had the exactly same subtile issue, a regression, because both req.path or req.url.pathname do not contain the old path (including the search parameters).

Thanks for pointing that out.

@rluba
Copy link
Contributor

rluba commented Feb 27, 2019

@framerate I’ve just published a PR that fixes your issue with bell (hapijs/bell#397).

AdriVanHoudt pushed a commit to hapijs/bell that referenced this issue Feb 27, 2019
This PR includes two fixes related to [hapi 18](hapijs/hapi#3871):

1. Many tests broke because hapi now strips the default port from `request.info.host` (a side effect of using the WHATWG URL API) – causing lots of assertions to fail. I decided to use a non-default port in the tests instead of removing the default port from the assertions. This verifies that port info is still propagated correctly.
2. `request.url.query` is no longer available. I’ve changed it to `request.query` which works for hapi 18 and older versions. (I’ve tested it with hapi 17 and 18). This fixes #394.
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking changes Change that can breaking existing code release notes Major release documentation
Projects
None yet
Development

No branches or pull requests

5 participants