Skip to content

Add workaround for loopback#643

Merged
rishabh merged 5 commits into
masterfrom
rish/loopback-workaround
Aug 9, 2019
Merged

Add workaround for loopback#643
rishabh merged 5 commits into
masterfrom
rish/loopback-workaround

Conversation

@rishabh

@rishabh rishabh commented Aug 7, 2019

Copy link
Copy Markdown
Contributor

What does this PR do?

This PR adds a workaround to support the loopback library. Since loopback tries to find the express layers by comparing handler functions, it can't find the layers since we wrap the handlers. This causes the router stack to be filled with entries that are un-sortable. This is an issue because adding a new layer will cause the layer to be put at the bottom of the stack, below the raiseUrlNotFoundError layer, even after sorting. That means requests will throw a 404 error even though the route is properly registered.

This PR makes use of loopback's aggressive search for the original handle function by adding a reference to the original handle function.

Motivation

Support loopback.

@rochdev rochdev added this to the 0.15.0 milestone Aug 9, 2019
@rishabh
rishabh merged commit 12f3dd8 into master Aug 9, 2019
@rishabh
rishabh deleted the rish/loopback-workaround branch August 9, 2019 15:26
rochdev pushed a commit that referenced this pull request Aug 26, 2019
* Add reference to original handler
BridgeAR added a commit that referenced this pull request Jun 25, 2026
This makes sure loopback's phase-based middleware sorting keeps working under
tracing. loopback's `_findLayerByHandler` maps an express layer back to the
user handler by scanning the layer handle's enumerable properties, then tags
the layer with its phase. Replacing `layer.handle` with the tracing wrapper hid
the original handler, so the lookup failed, the layer kept no phase, and an
`app.middleware('final', ...)` handler ran in insertion order ahead of the
route handlers instead of last. Restoring the `_datadog_orig` back-reference
that the router instrumentation rewrite dropped lets the lookup succeed again.

Refs: #643

* test(express): run the silently-skipped loopback suite

1. The block passed the plugin export to `withVersions` instead of the
   `'express'` integration name, so no installed version matched and the suite
   never ran. Switching to the name runs it.
2. Legacy loopback emits the deprecated `util._extend` warning at module load,
   which the harness deprecation guard turns into a thrown error; allowlist that
   one message so the modules load.
dd-octo-sts Bot pushed a commit that referenced this pull request Jun 26, 2026
This makes sure loopback's phase-based middleware sorting keeps working under
tracing. loopback's `_findLayerByHandler` maps an express layer back to the
user handler by scanning the layer handle's enumerable properties, then tags
the layer with its phase. Replacing `layer.handle` with the tracing wrapper hid
the original handler, so the lookup failed, the layer kept no phase, and an
`app.middleware('final', ...)` handler ran in insertion order ahead of the
route handlers instead of last. Restoring the `_datadog_orig` back-reference
that the router instrumentation rewrite dropped lets the lookup succeed again.

Refs: #643

* test(express): run the silently-skipped loopback suite

1. The block passed the plugin export to `withVersions` instead of the
   `'express'` integration name, so no installed version matched and the suite
   never ran. Switching to the name runs it.
2. Legacy loopback emits the deprecated `util._extend` warning at module load,
   which the harness deprecation guard turns into a thrown error; allowlist that
   one message so the modules load.
IlyasShabi pushed a commit that referenced this pull request Jun 26, 2026
This makes sure loopback's phase-based middleware sorting keeps working under
tracing. loopback's `_findLayerByHandler` maps an express layer back to the
user handler by scanning the layer handle's enumerable properties, then tags
the layer with its phase. Replacing `layer.handle` with the tracing wrapper hid
the original handler, so the lookup failed, the layer kept no phase, and an
`app.middleware('final', ...)` handler ran in insertion order ahead of the
route handlers instead of last. Restoring the `_datadog_orig` back-reference
that the router instrumentation rewrite dropped lets the lookup succeed again.

Refs: #643

* test(express): run the silently-skipped loopback suite

1. The block passed the plugin export to `withVersions` instead of the
   `'express'` integration name, so no installed version matched and the suite
   never ran. Switching to the name runs it.
2. Legacy loopback emits the deprecated `util._extend` warning at module load,
   which the harness deprecation guard turns into a thrown error; allowlist that
   one message so the modules load.
pabloerhard pushed a commit that referenced this pull request Jun 26, 2026
This makes sure loopback's phase-based middleware sorting keeps working under
tracing. loopback's `_findLayerByHandler` maps an express layer back to the
user handler by scanning the layer handle's enumerable properties, then tags
the layer with its phase. Replacing `layer.handle` with the tracing wrapper hid
the original handler, so the lookup failed, the layer kept no phase, and an
`app.middleware('final', ...)` handler ran in insertion order ahead of the
route handlers instead of last. Restoring the `_datadog_orig` back-reference
that the router instrumentation rewrite dropped lets the lookup succeed again.

Refs: #643

* test(express): run the silently-skipped loopback suite

1. The block passed the plugin export to `withVersions` instead of the
   `'express'` integration name, so no installed version matched and the suite
   never ran. Switching to the name runs it.
2. Legacy loopback emits the deprecated `util._extend` warning at module load,
   which the harness deprecation guard turns into a thrown error; allowlist that
   one message so the modules load.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants