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

KOJO-179 | throw a LogicException instead of silently not updating the state #103

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ COPY . $PROJECT_DIR

# Copy xdebug configration for remote debugging
COPY docker/xdebug.ini /usr/local/etc/php/conf.d/xdebug.ini
COPY docker/bwilson.ini /usr/local/etc/php/conf.d/bwilson.ini

RUN bash docker/build.sh \
--composer-token ${COMPOSER_TOKEN} \
Expand Down
2 changes: 2 additions & 0 deletions src/Api/V1/Worker/Service.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ public function applyRequest(): ServiceInterface
throw new \UnexpectedValueException('Unexpected value[' . $this->_read(self::PROP_REQUEST) . '].');
}
$this->_create(self::PROP_REQUEST_APPLIED, true);
} else {
throw new \LogicException('A request has already been applied, and cannot be updated');
}

return $this;
Expand Down
30 changes: 8 additions & 22 deletions src/Foreman.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ class Foreman implements ForemanInterface
use Locator\AwareTrait;
use Update\Work\Factory\AwareTrait;
use Update\Panic\Factory\AwareTrait;
use Update\Crash\Factory\AwareTrait;
use Update\Complete\Success\Factory\AwareTrait;
use Defensive\AwareTrait;
use Logger\AwareTrait;
Expand All @@ -46,11 +45,15 @@ protected function _workWorker(): ForemanInterface
try {
$this->getProcessPoolLoggerMessageMetadataBuilder()->setJob($this->_getJob());
$this->_updateJobAsWorking();
restore_error_handler();
$this->_runWorker();
set_error_handler(new ErrorHandler());
$this->_updateJobAfterWork();
} catch (Locator\Exception | \Error $throwable) {
} catch (\Throwable $throwable) {
set_error_handler(new ErrorHandler());
$this->_panicJob();
$jobId = $this->_getJob()->getId();
// exiting with nonzero code will give the Root's SIGCHLD handler some info
throw new \RuntimeException("Panicking job with ID[$jobId].", 0, $throwable);
}
$this->_getSemaphore()->releaseLock($this->_getNewJobOwnerResource($this->_getJob()));
Expand Down Expand Up @@ -84,17 +87,9 @@ protected function _injectRDBMSConnectionService(): ForemanInterface

protected function _runWorker(): ForemanInterface
{
try {
restore_error_handler();
$this->_injectWorkerService();
$this->_injectRDBMSConnectionService();
call_user_func($this->_getLocator()->getCallable());
set_error_handler(new ErrorHandler());
} catch (\Exception $exception) {
set_error_handler(new ErrorHandler());
$this->_crashJob();
throw $exception;
}
$this->_injectWorkerService();
$this->_injectRDBMSConnectionService();
call_user_func($this->_getLocator()->getCallable());

return $this;
}
Expand Down Expand Up @@ -152,13 +147,4 @@ protected function _panicJob(): ForemanInterface

return $this;
}

protected function _crashJob(): ForemanInterface
{
$updateCrash = $this->_getServiceUpdateCrashFactory()->create();
$updateCrash->setJob($this->_getJob());
$updateCrash->save();

return $this;
}
}
1 change: 0 additions & 1 deletion src/Foreman.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ services:
- [addSemaphoreResourceFactory, ['@semaphore.resource.factory-job']]
- [setServiceUpdateWorkFactory, ['@service.update.work.factory']]
- [setServiceUpdatePanicFactory, ['@service.update.panic.factory']]
- [setServiceUpdateCrashFactory, ['@service.update.crash.factory']]
- [setServiceUpdateCompleteSuccessFactory, ['@service.update.complete.success.factory']]
- [setLogger, ['@process.pool.logger']]
- [setProcessPoolLoggerMessageMetadataBuilder, ['@neighborhoods.kojo.process.pool.logger.message.metadata.builder']]
Expand Down
4 changes: 1 addition & 3 deletions src/ForemanInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,5 @@ public function workWorker(): ForemanInterface;

public function setServiceUpdateWorkFactory(Update\Work\FactoryInterface $updateWorkFactory);

public function setServiceUpdateCrashFactory(Update\Crash\FactoryInterface $updateCrashFactory);

public function setApiV1WorkerService(ServiceInterface $workerService);
}
}
84 changes: 0 additions & 84 deletions src/Process/Pool/Logger/Message.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,26 +23,6 @@ class Message implements MessageInterface, \JsonSerializable
protected $context_json_last_error;
/** @var MetadataInterface */
protected $kojo_metadata;
/**
* @var int
* @deprecated
*/
protected $process_id;
/**
* @var string
* @deprecated
*/
protected $process_path;
/**
* @var JobInterface
* @deprecated
*/
protected $kojo_job;
/**
* @var SerializableProcessInterface
* @deprecated
*/
protected $kojo_process;

public function jsonSerialize(): array
{
Expand Down Expand Up @@ -168,68 +148,4 @@ public function setMetadata(MetadataInterface $kojo_metadata) : MessageInterface

return $this;
}

/**
* @param int $process_id
* @return MessageInterface
* @deprecated
*/
public function setProcessId(int $process_id) : MessageInterface
{
if ($this->process_id !== null) {
throw new \LogicException('Message process_id is already set.');
}

$this->process_id = $process_id;

return $this;
}

/**
* @param string $process_path
* @return MessageInterface
* @deprecated
*/
public function setProcessPath(string $process_path) : MessageInterface
{
if ($this->process_path !== null) {
throw new \LogicException('Message process_path is already set.');
}

$this->process_path = $process_path;

return $this;
}

/**
* @param JobInterface $kojo_job
* @return MessageInterface
* @deprecated
*/
public function setKojoJob(JobInterface $kojo_job) : MessageInterface
{
if ($this->kojo_job !== null) {
throw new \LogicException('Message kojo_job is already set.');
}

$this->kojo_job = $kojo_job;

return $this;
}

/**
* @param SerializableProcessInterface $kojo_process
* @return MessageInterface
* @deprecated
*/
public function setKojoProcess(SerializableProcessInterface $kojo_process) : MessageInterface
{
if ($this->kojo_process !== null) {
throw new \LogicException('Message kojo_process is already set.');
}

$this->kojo_process = $kojo_process;

return $this;
}
}
14 changes: 2 additions & 12 deletions src/Process/Pool/Logger/Message/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,8 @@ public function build() : MessageInterface

$logMessage->setContextJsonLastError(json_last_error());

$metadata = $this->getProcessPoolLoggerMessageMetadataBuilder()->build();
$logMessage->setMetadata($metadata);

if ($metadata->hasJob()) {
$logMessage->setKojoJob($metadata->getJob());
}

if ($metadata->hasProcess()) {
$logMessage->setKojoProcess($metadata->getProcess());
$logMessage->setProcessId($metadata->getProcess()->getProcessId());
$logMessage->setProcessPath($metadata->getProcess()->getPath());
}
$kojoMetadata = $this->getProcessPoolLoggerMessageMetadataBuilder()->build();
$logMessage->setMetadata($kojoMetadata);

return $logMessage;
}
Expand Down
26 changes: 0 additions & 26 deletions src/Process/Pool/Logger/MessageInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,30 +33,4 @@ public function setContextJsonLastError(int $context_json_last_error): MessageIn
public function setMetadata(MetadataInterface $kojo_metadata) : MessageInterface;

public function getMetadata() : MetadataInterface;

/** @param int $process_id
* @return MessageInterface
* @deprecated
*/
public function setProcessId(int $process_id) : MessageInterface;

/** @param string $process_path
* @return MessageInterface
* @deprecated
*/
public function setProcessPath(string $process_path) : MessageInterface;

/**
* @param JobInterface $kojo_job
* @return MessageInterface
* @deprecated
*/
public function setKojoJob(JobInterface $kojo_job) : MessageInterface;

/**
* @param SerializableProcessInterface $kojo_job
* @return MessageInterface
* @deprecated
*/
public function setKojoProcess(SerializableProcessInterface $kojo_job) : MessageInterface;
}