Extension function calls safety #556
Comments
As reported in #556, when the extension is not installed or fails to load, calls to certain extension functions will fail. The issue mentions the `runningInContainer` broken being broken on the main branch. PR #554 also removes some safety checks that cause the same issue for the `diagnoseRaw` function. Those safety checks were removed because they did not what expected, tracking if the extension was running or not. For both functions I've implemented a no-operations function. This will fix any errors on calls to these functions. Other functions that call these functions need to handle receiving no return value. Part of #556, but doesn't fix it entirely. We should call more function calls. These ones were the most obvious ones. Based on PR #554.
As reported in #556, when the extension is not installed or fails to load, calls to certain extension functions will fail. The issue mentions the `runningInContainer` broken being broken on the main branch. PR #554 also removes some safety checks that cause the same issue for the `diagnoseRaw` function. Those safety checks were removed because they did not what expected, tracking if the extension was running or not. For both functions I've implemented a no-operations function. This will fix any errors on calls to these functions. Other functions that call these functions need to handle receiving no return value. Part of #556, but doesn't fix it entirely. We should call more function calls. These ones were the most obvious ones. Based on PR #554.
As reported in #556, when the extension is not installed or fails to load, calls to certain extension functions will fail. The issue mentions the `runningInContainer` broken being broken on the main branch. PR #554 also removes some safety checks that cause the same issue for the `diagnoseRaw` function. Those safety checks were removed because they did not what expected, tracking if the extension was running or not. For both functions I've implemented a no-operations function. This will fix any errors on calls to these functions. Other functions that call these functions need to handle receiving no return value. Part of #556, but doesn't fix it entirely. We should call more function calls. These ones were the most obvious ones. Based on PR #554.
As reported in #556, when the extension is not installed or fails to load, calls to certain extension functions will fail. The issue mentions the `runningInContainer` broken being broken on the main branch. PR #554 also removes some safety checks that cause the same issue for the `diagnoseRaw` function. Those safety checks were removed because they did not what expected, tracking if the extension was running or not. For both functions I've implemented a no-operations function. This will fix any errors on calls to these functions. Other functions that call these functions need to handle receiving no return value. Part of #556, but doesn't fix it entirely. We should call more function calls. These ones were the most obvious ones. Based on PR #554.
As reported in #556, when the extension is not installed or fails to load, calls to certain extension functions will fail. The issue mentions the `runningInContainer` broken being broken on the main branch. PR #554 also removes some safety checks that cause the same issue for the `diagnoseRaw` function. Those safety checks were removed because they did not what expected, tracking if the extension was running or not. For both functions I've implemented a no-operations function. This will fix any errors on calls to these functions. Other functions that call these functions need to handle receiving no return value. Part of #556, but doesn't fix it entirely. We should call more function calls. These ones were the most obvious ones. Based on PR #554.
As reported in #556, when the extension is not installed or fails to load, calls to certain extension functions will fail. The issue mentions the `runningInContainer` broken being broken on the main branch. PR #554 also removes some safety checks that cause the same issue for the `diagnoseRaw` function. Those safety checks were removed because they did not what expected, tracking if the extension was running or not. For both functions I've implemented a no-operations function. This will fix any errors on calls to these functions. Other functions that call these functions need to handle receiving no return value. Part of #556, but doesn't fix it entirely. We should call more function calls. These ones were the most obvious ones. Based on PR #554.
As reported in #556, when the extension is not installed or fails to load, calls to certain extension functions will fail. The issue mentions the `runningInContainer` broken being broken on the main branch. PR #554 also removes some safety checks that cause the same issue for the `diagnoseRaw` function. Those safety checks were removed because they did not what expected, tracking if the extension was running or not. For both functions I've implemented a no-operations function. This will fix any errors on calls to these functions. Other functions that call these functions need to handle receiving no return value. Part of #556, but doesn't fix it entirely. We should call more function calls. These ones were the most obvious ones. Based on PR #554.
As reported in #556, when the extension is not installed or fails to load, calls to certain extension functions will fail. The issue mentions the `runningInContainer` broken being broken on the main branch. PR #554 also removes some safety checks that cause the same issue for the `diagnoseRaw` function. Those safety checks were removed because they did not what expected, tracking if the extension was running or not. For both functions I've implemented a no-operations function. This will fix any errors on calls to these functions. Other functions that call these functions need to handle receiving no return value. Part of #556, but doesn't fix it entirely. We should call more function calls. These ones were the most obvious ones. Based on PR #554.
As reported in #556, when the extension is not installed or fails to load, calls to certain extension functions will fail. The issue mentions the `runningInContainer` broken being broken on the main branch. PR #554 also removes some safety checks that cause the same issue for the `diagnoseRaw` function. Those safety checks were removed because they did not what expected, tracking if the extension was running or not. For both functions I've implemented a no-operations function. This will fix any errors on calls to these functions. Other functions that call these functions need to handle receiving no return value. Part of #556, but doesn't fix it entirely. We should call more function calls. These ones were the most obvious ones. Based on PR #554.
As reported in #556, when the extension is not installed or fails to load, calls to certain extension functions will fail. The issue mentions the `runningInContainer` broken being broken on the main branch. PR #554 also removes some safety checks that cause the same issue for the `diagnoseRaw` function. Those safety checks were removed because they did not what expected, tracking if the extension was running or not. For both functions I've implemented a no-operations function. This will fix any errors on calls to these functions. Other functions that call these functions need to handle receiving no return value. Part of #556, but doesn't fix it entirely. We should call more function calls. These ones were the most obvious ones. Based on PR #554.
|
The Node.js integration doesn't impelement "noop" functions for all extension functions like the other integrations do. That may be a safer way to solve this issue, and doesn't require checks wherever we call the extension functions. Creating a |
|
I need to check all the places these attributes on the ExtensionWrapper can get called. The extension wrapper seems to be the most direct way to interact with the extension. It looks like the Span extension interaction is all hidden away through noop objects if it's not available. It's different than what I'm used to from the Elixir and Ruby integrations, where we stub out the extension functions and leave all the other functionality intact. It now is is a little difficult to follow sometimes what the data flow is, when the extension isn't loaded. We can choose to remove all the noop objects in favor of only stubbing out the extension functions if that's easier. My journey notes so far: The BaseSpan object is the only thing that interacts with the "span" extension class. Which we then have to rely on, that nothing calls that BaseSpan ever, if the extension is not loaded. If the extension isn't loaded, it will return It can return "NoopSpan"s here if no (root) span is currently active. This check reads a little weird. It uses the noopspan as a nil check if no extension is present, while it's also used when the extension isn't loaded. We should rather ask "is there a root span? If no, create a new one". Move that behavior to the tracer or communicate it with another function call than like this. |
|
I wanted to test a small refactor, taking out all the |
|
Created a separate issue to first improve test coverage before I can reliably try this refactor. #605 |
When the extension fails to install, or has been installed for a not matching architecture, function calls to the extension may fail.
For example, when running the macOS build on a Linux docker image, calling the diagnose function results in the following:
(Example run on the main branch before PR #554 is merged. 58f1ae0)
This shouldn't happen. It shouldn't cause an error.
When the extension module is not present it should not call functions that may fail.
To do
runningInContaineras in the example. PR Fix extension function fallbacks #557Double check any other extension function calls for safety when the extension is not loaded.The text was updated successfully, but these errors were encountered: