search

bluehalo / node-fhir-server-core

An Open Source secure REST implementation for the HL7 FHIR Specification. For API documentation, please see https://github.com/Asymmetrik/node-fhir-server-core/wiki.
https://asymmetrik.com/healthcare
MIT License
391 stars 120 forks source link

Getting started doesn't work; *phoenix.ico* not found #182

Closed aroder closed 5 years ago

aroder commented 5 years ago

Do you want to request a feature, report a bug, or improve documentation?

Bug

If you are reporting a bug?

Bug

What is the current behavior? Walkthrough fails. The initialize function cannot locate phoenix.ico.

image

What is the expected behavior? Expected server to start on port 3000

What are the steps to reproduce? Perform the getting started walkthrough. After adding the patient profile and updating the config in index.js with the version, attempt to start the server.

What OS are you using and what version of node.js and @asymmetrik/node-fhir-server-core are you running?

OS: Windows node version: 8.11.1 server version: 2.0.2

Robert-W commented 5 years ago

Thanks @aroder for reporting this. We'll take a look. It looks like the relative path might not be correct in your case. It should read cquentia-fhir-server/src/assets/phoenix.ico instead of cquentia-fhir-server/assets/phoenix.ico (looks like it is missing the src folder). This was working on my development laptop but it is a mac, so I'll do some more testing to see if I can figure out whats happening.

aroder commented 5 years ago

Thanks @Robert-W. I ended up manually copying the file to the location where the framework is looking for it. That gets me by for the moment. Let me know if I can assist in testing

Robert-W commented 5 years ago

Ok thanks, could you check something for me. In src/server/server.js, on line 148, you should see something like this: 

this.app.use(favicon(this.config.server.favicon || path.posix.join(__dirname, '../assets/phoenix.ico')));

Can you add this snippet directly beneath it and let me know what the output is?

console.log(path.posix.join(__dirname, '../assets/phoenix.ico'));

It should look something like this /Users/<username>/Projects/fhir/node-fhir-server-core/src/assets/phoenix.ico, but I have a hunch it's missing the src directory in yours which means the path.join is not working.

aroder commented 5 years ago

Yeah, that seems to be the problem. Output is just assets/phoenix.ico.

I did confirm that __dirname is populated with ...\node_modules\@asymmetrik\node-fhir-server-core\src\server

I'm seeing this on a windows machine. Is there any reason you have to use path.posix.join instead of just path.join? path.posix goes to POSIX-specific implementations of functions like join. https://nodejs.org/api/path.html#path_path_posix

-- Adam Roderick 720-940-3512 aroder@gmail.com

On Fri, Jun 14, 2019 at 7:38 AM Robert notifications@github.com wrote:

Ok thanks, could you check something for me. In src/server/server.js, on line 148, you should see something like this:

this.app.use(favicon(this.config.server.favicon || path.posix.join(__dirname, '../assets/phoenix.ico')));

Can you add this snippet directly beneath it and let me know what the output is?

console.log(path.posix.join(__dirname, '../assets/phoenix.ico'));

It should look something like this /Users//Projects/fhir/node-fhir-server-core/src/assets/phoenix.ico, but I have a hunch it's missing the src directory in yours which means the path.join is not working.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Asymmetrik/node-fhir-server-core/issues/182?email_source=notifications&email_token=AAAFYYVENYEJ4OZSNVXWMDLP2ONNNA5CNFSM4HWT2N6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXW2CVA#issuecomment-502112596, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAFYYVMUA6ARAG4DU2XOVLP2ONNNANCNFSM4HWT2N6A .

Robert-W commented 5 years ago

Hi @aroder, sorry for the long delayed response, things were a little hectic the last 30 days. This was fixed but i believe we need to publish an updated package. I will see if we can't get something out soon.

We had originally added posix for something, but I can't remember (might have been CI but don't hold me to that). It's now removed. I'll close this ticket when we publish a new release.

aroder commented 5 years ago

Thanks! We had a reasonable workaround in the meantime

-- Adam Roderick 720-940-3512 aroder@gmail.com

On Mon, Aug 12, 2019 at 8:42 AM Robert notifications@github.com wrote:

Hi @aroder https://github.com/aroder, sorry for the long delayed response, things were a little hectic the last 30 days. This was fixed but i believe we need to publish an updated package. I will see if we can't get something out soon.

We had originally added posix for something, but I can't remember (might have been CI but don't hold me to that). It's now removed. I'll close this ticket when we publish a new release.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Asymmetrik/node-fhir-server-core/issues/182?email_source=notifications&email_token=AAAFYYXDJRF2WECTZDV77VDQEFZF5A5CNFSM4HWT2N6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4CYAYY#issuecomment-520454243, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAFYYUVTVDRHUVJOHL5IJTQEFZF5ANCNFSM4HWT2N6A .

Robert-W commented 5 years ago

No problem and glad we weren't holding you up, sorry again for the delays

mduna commented 5 years ago

In windows 10, I have to take posix out to make the error go away. in file server.js line 148. //this.app.use(favicon(this.config.server.favicon || path.posix.join(dirname, '../assets/phoenix.ico'))); this.app.use(favicon(this.config.server.favicon || path.join(dirname, '../assets/phoenix.ico')));

Robert-W commented 5 years ago

Yes, you can see more on the details above, there will be a release out this week that fixes this issue.

mduna commented 5 years ago

I am using version 2.0.3, I got it yesterday. Are you sure the fix is there? Thanks.

Robert-W commented 5 years ago

Yes, however, we have not published the version to npm which is slated for early this week. We are trying to wrap up testing on this and several other things at the same time so once it's approved, we will publish a release.

You can yarn add the master branch, but I would not recommend that. Once we publish a new npm version, you will be able to install 2.0.4.

mduna commented 5 years ago

Thank you for explaining it. I was wondering why I did not get the fix. I can wait for version 2.0.4.

Robert-W commented 5 years ago

no problem, I'll update this ticket and close it once it is published.

Robert-W commented 5 years ago

@mduna @aroder 2.0.4 is published