Skip to content

Commit

Permalink
Fixes bug introduced in v0.7.2 wherein matlab-proxy would crash whe…
Browse files Browse the repository at this point in the history
…n MATLAB is not on the path.

Introduces new error messages when MATLAB is not found on path or when specified incorrectly using the MWI_CUSTOM_MATLAB_ROOT environment variable.
  • Loading branch information
Prabhakar Kumar committed Aug 7, 2023
1 parent 01dd376 commit 055e519
Show file tree
Hide file tree
Showing 14 changed files with 296 additions and 244 deletions.
4 changes: 2 additions & 2 deletions gui/src/components/App/App.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ describe('App Component', () => {
});


const paragraphElements = [...container.getElementsByTagName('p')];
const paragraphElements = [...container.getElementsByTagName('pre')];


expect(
Expand All @@ -173,7 +173,7 @@ describe('App Component', () => {
initialState: initialState,
});

const paragraphElements = [...container.getElementsByTagName('p')];
const paragraphElements = [...container.getElementsByTagName('pre')];

expect(
paragraphElements.some((p) =>
Expand Down
11 changes: 9 additions & 2 deletions gui/src/components/Error/Error.css
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
/* Copyright (c) 2020-2022 The MathWorks, Inc. */
/* Copyright (c) 2020-2023 The MathWorks, Inc. */

#error .alert {
padding: 0;
margin-bottom: 0;
border-radius: 10px;
}

#error .modal-body,
#error .modal-body pre {
background-color: hsla(0, 0%, 100%, 0);
border: none;
font-family: inherit;
font-size: 15px;
white-space: pre-wrap;
}

#error .modal-header {
padding-left: 45px;
}
Expand Down
4 changes: 2 additions & 2 deletions gui/src/components/Error/Error.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2020-2022 The MathWorks, Inc.
// Copyright (c) 2020-2023 The MathWorks, Inc.

import React from 'react';
import Error from './index';
Expand All @@ -22,7 +22,7 @@ describe('Error Component', () => {
it('should render without crashing ', () => {
const { container } = render(<Error message={message} />);

const paragraphs = [...container.getElementsByTagName('p')];
const paragraphs = [...container.getElementsByTagName('pre')];

expect(paragraphs.some((p) => p.textContent === message)).toBeTruthy();
});
Expand Down
4 changes: 2 additions & 2 deletions gui/src/components/Error/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2020-2022 The MathWorks, Inc.
// Copyright (c) 2020-2023 The MathWorks, Inc.

import React from 'react';
import PropTypes from 'prop-types';
Expand Down Expand Up @@ -28,7 +28,7 @@ function Error({ message, logs, children }) {
<h4 className="modal-title alert_heading">Error</h4>
</div>
<div className="modal-body">
<p>{message}</p>
<pre>{message}</pre>
<Linkify>{logReport}</Linkify>
{children}
</div>
Expand Down
11 changes: 7 additions & 4 deletions matlab_proxy/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@
from matlab_proxy.util import list_servers, mwi
from matlab_proxy.util.mwi import environment_variables as mwi_env
from matlab_proxy.util.mwi import token_auth
from matlab_proxy.util.mwi.exceptions import AppError, InvalidTokenError, LicensingError
from matlab_proxy.util.mwi.exceptions import (
AppError,
InvalidTokenError,
LicensingError,
)

mimetypes.add_type("font/woff", ".woff")
mimetypes.add_type("font/woff2", ".woff2")
Expand Down Expand Up @@ -109,12 +113,12 @@ async def create_status_response(app, loadUrl=None):
{
"matlab": {
"status": await state.get_matlab_state(),
"version": state.settings["matlab_version"],
"version": state.settings.get("matlab_version", "Unknown"),
},
"licensing": marshal_licensing_info(state.licensing),
"loadUrl": loadUrl,
"error": marshal_error(state.error),
"wsEnv": state.settings["ws_env"],
"wsEnv": state.settings.get("ws_env", ""),
}
)

Expand Down Expand Up @@ -160,7 +164,6 @@ async def authenticate_request(req):
Returns:
JSONResponse: JSONResponse object containing information about authentication status and error if any.
"""
state = req.app["state"]
if await token_auth.authenticate_request(req):
logger.debug("!!!!!! REQUEST IS AUTHORIZED !!!!")
authStatus = True
Expand Down
49 changes: 11 additions & 38 deletions matlab_proxy/app_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@
from matlab_proxy.util.mwi.exceptions import (
EmbeddedConnectorError,
EntitlementError,
InternalError,
FatalError,
LicensingError,
MatlabError,
MatlabInstallError,
OnlineLicensingError,
XvfbError,
log_error,
Expand Down Expand Up @@ -70,11 +69,12 @@ def __init__(self, settings):
self.logs = {
"matlab": deque(maxlen=200),
}
self.error = None
# Start in an error state if MATLAB is not present
if not self.is_matlab_present():
self.error = MatlabInstallError("'matlab' executable not found in PATH")
logger.error("'matlab' executable not found in PATH")

# Initialize with the error state from the initialization of settings
self.error = settings["error"]

if self.error is not None:
self.logs["matlab"].clear()
return

# Keep track of when the Embedded connector starts.
Expand Down Expand Up @@ -153,8 +153,8 @@ async def init_licensing(self):
self.__delete_cached_licensing_file()

# NLM Connection String set in environment
elif self.settings["nlm_conn_str"] is not None:
nlm_licensing_str = self.settings["nlm_conn_str"]
elif self.settings.get("nlm_conn_str", None) is not None:
nlm_licensing_str = self.settings.get("nlm_conn_str")
logger.debug(f"Found NLM:[{nlm_licensing_str}] set in environment")
logger.debug(f"Using NLM string to connect ... ")
self.licensing = {
Expand Down Expand Up @@ -395,26 +395,17 @@ def is_licensed(self):
return True
return False

def is_matlab_present(self):
"""Is MATLAB install accessible?
Returns:
Boolean: True if MATLAB is present in the system. False otherwise.
"""

return self.settings["matlab_path"] is not None

async def update_entitlements(self):
"""Speaks to MW and updates MHLM entitlements
Raises:
InternalError: When licensing is None or when licensing type is not MHLM.
FatalError: When licensing is None or when licensing type is not MHLM.
Returns:
Boolean: True if update was successful
"""
if self.licensing is None or self.licensing["type"] != "mhlm":
raise InternalError(
raise FatalError(
"MHLM licensing must be configured to update entitlements!"
)

Expand Down Expand Up @@ -722,26 +713,8 @@ async def start_matlab(self, restart_matlab=False):
Args:
restart_matlab (bool, optional): Whether to restart MATLAB. Defaults to False.
Raises:
Exception: When MATLAB is already running and restart is False.
Exception: When MATLAB is not licensed.
"""

# FIXME
if await self.get_matlab_state() != "down" and restart_matlab is False:
raise Exception("MATLAB already running/starting!")

# FIXME
if not self.is_licensed():
raise Exception("MATLAB is not licensed!")

if not self.is_matlab_present():
self.error = MatlabInstallError("'matlab' executable not found in PATH")
logger.error("'matlab' executable not found in PATH")
self.logs["matlab"].clear()
return

# Ensure that previous processes are stopped
await self.stop_matlab()

Expand Down
Loading

0 comments on commit 055e519

Please sign in to comment.